MainWindow: Fix crash for quick image changes

[palinek]

fixes #94

[npmiller]

That should help but it's still not really safe, the other thread could definitely change loadJob_ to nullptr between the if and this line. And this thread could also set it to nullptr between the nullptr check and the cancel in the other thread.

Even if you were also using sender() on this line I believe the other accesses to loadJob_ (== and = nullptr) would still technically be undefined behaviour. We need to either make sure we don't need to access it concurrently or just lock it with a mutex when we do.

[palinek]

the other thread could definitely change loadJob_ to nullptr between the if and this line.

Nope, the "job" thread doesn't change the loadJob_ member anyhow. It is purely handled in the MainWindow and the main thread. Also the LoadJob objects are created in the main thread.

The race solved here was as follows: the "job" thread is running and user wants to load a new picture, so the the MainWindow cancels the running job and creates a new one. But in the meanwhile the old load "job" is finished (before it could have been cancelled) and emitted the finished() signal (so the onImageLoaded() is queued to event processing loop into the main thread). Ten onImageLoaded() is executed and loadJob_ is set to nullptr. When the new load "job" is finished and the onImageLoaded() executed.... the loadJob_ is nullptr and the crash happens.

[npmiller]

Oh I see that makes sense, because of the behaviour I was under the impression that onImageLoaded() ended up running in the job thread somehow. But I see how it could happen with the behaviour you describe. I can't test it right now but if that's actually what's going on then your fix is good.

+1

[agaida]

GTM