Crash when loading consecutive images quickly about lximage-qt

[tsujan]

Confirmed:

#0  0x00007fecfa738dbe in QImage::QImage(QImage const&) () at /usr/lib/libQt5Gui.so.5
#1  0x000000000041b921 in  ()
#2  0x00007fecfa1bae72 in QObject::event(QEvent*) () at /usr/lib/libQt5Core.so.5
#3  0x00007fecfaef7473 in QWidget::event(QEvent*) () at /usr/lib/libQt5Widgets.so.5
#4  0x00007fecfafe8b7b in QMainWindow::event(QEvent*) () at /usr/lib/libQt5Widgets.so.5
#5  0x00007fecfaeb646c in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
    at /usr/lib/libQt5Widgets.so.5
#6  0x00007fecfaebdcf4 in QApplication::notify(QObject*, QEvent*) () at /usr/lib/libQt5Widgets.so.5
#7  0x00007fecfa18bb98 in QCoreApplication::notifyInternal2(QObject*, QEvent*) ()
    at /usr/lib/libQt5Core.so.5
#8  0x00007fecfa18e02d in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()
    at /usr/lib/libQt5Core.so.5
#9  0x00007fecfa1e3a63 in  () at /usr/lib/libQt5Core.so.5
#10 0x00007fecfb5fba47 in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0
#11 0x00007fecfb5fbc78 in  () at /usr/lib/libglib-2.0.so.0
#12 0x00007fecfb5fbd0c in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
#13 0x00007fecfa1e3e7f in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /usr/lib/libQt5Core.so.5
#14 0x00007fecfa18a21a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /usr/lib/libQt5Core.so.5
#15 0x00007fecfa192a74 in QCoreApplication::exec() () at /usr/lib/libQt5Core.so.5
#16 0x0000000000412761 in  ()
#17 0x00007fecfd2ad43a in __libc_start_main () at /usr/lib/libc.so.6
#18 0x0000000000412c7a in _start ()

At first, I though it might be related to Qt 5.9 but the Qt image viewer I've made for my personal use doesn't show this symptom.

[fulalas]

You're right, it's not a Qt 5.9.0 bug since I can reproduce this on Qt 5.8.0 too. Can be a Qt 5.x.x bug though :D

[tsujan]

Can be a Qt 5.x.x bug though

No, the bug is lximage-qt. I'll look into it whenever I find some free time.

[npmiller]

This is a race condition accessing the loadJob_ member of MainWindow, between loadImage (main thread) and onImageLoaded (job thread). The crash is because the when the job emits finished at a point after the main thread set loadJob_ to nullptr.

Disconnecting the finished signal from the job before cancelling it makes the issue a bit harder to hit but it's not a proper fix:

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 16191d9..f6e7c6f 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -583,7 +584,9 @@ void MainWindow::updateUI() {
 void MainWindow::loadImage(const Fm::FilePath & filePath, QModelIndex index) {
   // cancel loading of current image
   if(loadJob_) {
+    disconnect(loadJob_, &Fm::Job::finished, this, &MainWindow::onImageLoaded);
     loadJob_->cancel(); // the job object will be freed automatically later
     loadJob_ = nullptr;
   }
   if(imageModified_) {

Maybe loadJob_ could be guarded by a mutex, but maybe there's a nicer way to do it, before, onImageLoaded took a pointer to the job thus avoiding that issue, using QObject::sender could do the trick.

[palinek]

@npmiller Great analysis... in #95 I've made something rather similar as your suggestion.

[fulalas]

I can confirm that the new patch fixed it. I'm closing this issue.

Thanks, @npmiller and @palinek!

[palinek]

reopening... as the solution wasn't approved and merged into master yet