Try to fix thumbbrowser deadlock (#4576)

rtgui/filebrowserentry.cc

@@ -211,19 +211,8 @@ void FileBrowserEntry::procParamsChanged (Thumbnail* thm, int whoChangedIt)
 
 void FileBrowserEntry::updateImage (rtengine::IImage8* img, double scale, rtengine::procparams::CropParams cropParams)
 {
-
-    {
-        GThreadLock lock;
-
-        if ( feih == nullptr ||
-                feih->destroyed ) {
-            img->free();
-            return;
-        }
-
     redrawRequests++;
     feih->pending++;
-    }
 
     struct tiupdate {
         FileBrowserEntryIdleHelper* feih;

rtgui/filebrowserentry.h

@@ -19,23 +19,27 @@
 #ifndef _FILEBROWSERENTRY_
 #define _FILEBROWSERENTRY_
 
+#include <atomic>
+
 #include <gtkmm.h>
-#include "thumbbrowserentrybase.h"
-#include "thumbnail.h"
-#include "filethumbnailbuttonset.h"
-#include "thumbnaillistener.h"
-#include "thumbimageupdater.h"
-#include "imageareatoollistener.h"
-#include "editenums.h"
+
 #include "../rtengine/rtengine.h"
+
 #include "crophandler.h"
+#include "editenums.h"
+#include "filethumbnailbuttonset.h"
+#include "imageareatoollistener.h"
+#include "thumbbrowserentrybase.h"
+#include "thumbimageupdater.h"
+#include "thumbnail.h"
+#include "thumbnaillistener.h"
 
 
 class FileBrowserEntry;
 struct FileBrowserEntryIdleHelper {
     FileBrowserEntry* fbentry;
     bool destroyed;
-    int pending;
+    std::atomic<int> pending;
 };
 
 class FileThumbnailButtonSet;

rtgui/thumbbrowserentrybase.h

@@ -19,12 +19,15 @@
 #ifndef _THUMBNAILBROWSERENTRYBASE_
 #define _THUMBNAILBROWSERENTRYBASE_
 
+#include <atomic>
+
 #include <gtkmm.h>
-#include "lwbuttonset.h"
-#include "thumbnail.h"
-#include "threadutils.h"
-#include "guiutils.h"
+
 #include "cursormanager.h"
+#include "guiutils.h"
+#include "lwbuttonset.h"
+#include "threadutils.h"
+#include "thumbnail.h"
 
 class ThumbBrowserBase;
 class ThumbBrowserEntryBase
@@ -71,7 +74,7 @@ protected:
 
     int ofsX, ofsY; // offset due to the scrolling of the parent
 
-    int redrawRequests;
+    std::atomic<int> redrawRequests;
 
     ThumbBrowserBase* parent;
     ThumbBrowserEntryBase* original;

rtgui/thumbimageupdater.cc

@@ -17,9 +17,13 @@
  *  along with RawTherapee.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <atomic>
 #include <set>
-#include "thumbimageupdater.h"
+
 #include <gtkmm.h>
+
+#include "thumbimageupdater.h"
+
 #include "guiutils.h"
 #include "threadutils.h"
 
@@ -83,7 +87,7 @@ public:
 
     JobList jobs_;
 
-    unsigned int active_;
+    std::atomic<unsigned int> active_;
 
     bool inactive_waiting_;
 
@@ -157,13 +161,11 @@ public:
             j.listener_->updateImage(img, scale, thm->getProcParams().crop);
         }
 
-        {
+        if ( --active_ == 0 ) {
             Glib::Threads::Mutex::Lock lock(mutex_);
-
-            if ( --active_ == 0 &&
-                    inactive_waiting_ ) {
+            if (inactive_waiting_) {
                 inactive_waiting_ = false;
-                inactive_.signal();
+                inactive_.broadcast();
             }
         }
     }
@@ -181,8 +183,7 @@ ThumbImageUpdater::ThumbImageUpdater():
 {
 }
 
-void
-ThumbImageUpdater::add(ThumbBrowserEntryBase* tbe, bool* priority, bool upgrade, ThumbImageUpdateListener* l)
+void ThumbImageUpdater::add(ThumbBrowserEntryBase* tbe, bool* priority, bool upgrade, ThumbImageUpdateListener* l)
 {
     // nobody listening?
     if ( l == nullptr ) {
@@ -216,11 +217,11 @@ ThumbImageUpdater::add(ThumbBrowserEntryBase* tbe, bool* priority, bool upgrade,
 }
 
 
-void
-ThumbImageUpdater::removeJobs(ThumbImageUpdateListener* listener)
+void ThumbImageUpdater::removeJobs(ThumbImageUpdateListener* listener)
 {
     DEBUG("removeJobs(%p)", listener);
 
+    {
         Glib::Threads::Mutex::Lock lock(impl_->mutex_);
 
         for( Impl::JobList::iterator i(impl_->jobs_.begin()); i != impl_->jobs_.end(); ) {
@@ -232,33 +233,35 @@ ThumbImageUpdater::removeJobs(ThumbImageUpdateListener* listener)
                 ++i;
             }
         }
+    }
 
     while ( impl_->active_ != 0 ) {
-        // XXX this is nasty... it would be nicer if we weren't called with
-        // this lock held
-        GThreadUnLock unlock;
         DEBUG("waiting for running jobs1");
+        {
+            Glib::Threads::Mutex::Lock lock(impl_->mutex_);
             impl_->inactive_waiting_ = true;
             impl_->inactive_.wait(impl_->mutex_);
         }
+    }
 }
 
-void
-ThumbImageUpdater::removeAllJobs()
+void ThumbImageUpdater::removeAllJobs()
 {
     DEBUG("stop");
 
+    {
         Glib::Threads::Mutex::Lock lock(impl_->mutex_);
 
         impl_->jobs_.clear();
+    }
 
     while ( impl_->active_ != 0 ) {
-        // XXX this is nasty... it would be nicer if we weren't called with
-        // this lock held
-        GThreadUnLock unlock;
         DEBUG("waiting for running jobs2");
+        {
+            Glib::Threads::Mutex::Lock lock(impl_->mutex_);
             impl_->inactive_waiting_ = true;
             impl_->inactive_.wait(impl_->mutex_);
         }
+    }
 }