From f9c0669c4231aeafd649ec2811f26f53e1ed8a14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fl=C3=B6ssie?= Date: Wed, 27 Jun 2018 20:11:18 +0200 Subject: [PATCH] Try to fix thumbbrowser deadlock (#4576) --- rtgui/filebrowserentry.cc | 15 +------- rtgui/filebrowserentry.h | 20 ++++++---- rtgui/thumbbrowserentrybase.h | 13 ++++--- rtgui/thumbimageupdater.cc | 69 ++++++++++++++++++----------------- 4 files changed, 58 insertions(+), 59 deletions(-) diff --git a/rtgui/filebrowserentry.cc b/rtgui/filebrowserentry.cc index cb3844687..04a5b2723 100644 --- a/rtgui/filebrowserentry.cc +++ b/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++; - } + redrawRequests++; + feih->pending++; struct tiupdate { FileBrowserEntryIdleHelper* feih; diff --git a/rtgui/filebrowserentry.h b/rtgui/filebrowserentry.h index 423128ddb..ada726dfc 100644 --- a/rtgui/filebrowserentry.h +++ b/rtgui/filebrowserentry.h @@ -19,23 +19,27 @@ #ifndef _FILEBROWSERENTRY_ #define _FILEBROWSERENTRY_ +#include + #include -#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 pending; }; class FileThumbnailButtonSet; diff --git a/rtgui/thumbbrowserentrybase.h b/rtgui/thumbbrowserentrybase.h index 0ebf597e2..8237b7c6b 100644 --- a/rtgui/thumbbrowserentrybase.h +++ b/rtgui/thumbbrowserentrybase.h @@ -19,12 +19,15 @@ #ifndef _THUMBNAILBROWSERENTRYBASE_ #define _THUMBNAILBROWSERENTRYBASE_ +#include + #include -#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 redrawRequests; ThumbBrowserBase* parent; ThumbBrowserEntryBase* original; diff --git a/rtgui/thumbimageupdater.cc b/rtgui/thumbimageupdater.cc index 696c68811..992b38812 100644 --- a/rtgui/thumbimageupdater.cc +++ b/rtgui/thumbimageupdater.cc @@ -17,9 +17,13 @@ * along with RawTherapee. If not, see . */ +#include #include -#include "thumbimageupdater.h" + #include + +#include "thumbimageupdater.h" + #include "guiutils.h" #include "threadutils.h" @@ -83,7 +87,7 @@ public: JobList jobs_; - unsigned int active_; + std::atomic 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,49 +217,51 @@ 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_); + { + Glib::Threads::Mutex::Lock lock(impl_->mutex_); - for( Impl::JobList::iterator i(impl_->jobs_.begin()); i != impl_->jobs_.end(); ) { - if (i->listener_ == listener) { - DEBUG("erasing specific job"); - Impl::JobList::iterator e(i++); - impl_->jobs_.erase(e); - } else { - ++i; + for( Impl::JobList::iterator i(impl_->jobs_.begin()); i != impl_->jobs_.end(); ) { + if (i->listener_ == listener) { + DEBUG("erasing specific job"); + Impl::JobList::iterator e(i++); + impl_->jobs_.erase(e); + } else { + ++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"); - impl_->inactive_waiting_ = true; - impl_->inactive_.wait(impl_->mutex_); + { + 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_); + { + Glib::Threads::Mutex::Lock lock(impl_->mutex_); - impl_->jobs_.clear(); + 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"); - impl_->inactive_waiting_ = true; - impl_->inactive_.wait(impl_->mutex_); + { + Glib::Threads::Mutex::Lock lock(impl_->mutex_); + impl_->inactive_waiting_ = true; + impl_->inactive_.wait(impl_->mutex_); + } } }