include/vcl/task.hxx | 2 +- vcl/README.scheduler | 14 ++++---------- vcl/inc/schedulerimpl.hxx | 17 ----------------- vcl/inc/svdata.hxx | 4 +++- vcl/source/app/scheduler.cxx | 22 ++-------------------- 5 files changed, 10 insertions(+), 49 deletions(-)
New commits: commit 69a9b48d50d98130a65aa6c823dc6cc464fefd71 Author: Stephan Bergmann <sberg...@redhat.com> AuthorDate: Tue Dec 8 16:04:48 2020 +0100 Commit: Stephan Bergmann <sberg...@redhat.com> CommitDate: Tue Dec 8 20:42:39 2020 +0100 Replace SchedulerMutex with (non-recursive) std::mutex ...following up on the TODO from 84af20ef3ea72190784e9e7be820684c2558ba8c "Make SchedulerMutex non-recursive" Change-Id: I3be98f2dba7c7486b79ec1f166431333cc69451a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107423 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sberg...@redhat.com> diff --git a/include/vcl/task.hxx b/include/vcl/task.hxx index 35efe4825bbc..d8adae7eff0b 100644 --- a/include/vcl/task.hxx +++ b/include/vcl/task.hxx @@ -91,7 +91,7 @@ public: /** * This function must be called for static tasks, so the Task destructor - * ignores the SchedulerMutex, as it may not be available anymore. + * ignores the scheduler mutex, as it may not be available anymore. * The cleanup is still correct, as it has already happened in * DeInitScheduler call well before the static destructor calls. */ diff --git a/vcl/README.scheduler b/vcl/README.scheduler index 466485f16645..e08f59e37cdf 100644 --- a/vcl/README.scheduler +++ b/vcl/README.scheduler @@ -60,7 +60,7 @@ Start new Tasks. The Scheduler just processes it's own Tasks in the main thread and needs the SolarMutex for it and for DeInit (tested by DBG_TESTSOLARMUTEX). All -the other interaction just take the SchedulerMutex or don't need locking +the other interaction just take the scheduler mutex or don't need locking at all. There is a "workaround" for static Task objecs, which would crash LO on @@ -69,9 +69,9 @@ Scheduler, while the SchedulerLock would be long gone. OTOH this makes Task handling less error-prone, than doing "special" manual cleanup. There is also a "= TODOs and ideas =" to get rid if static Tasks. -Actually the SchedulerMutex should never be locked when calling into -non-scheduler code, so it was converted to simulate a non-recursive -mutex later. As an alternative it could use a std::mutex. +Actually the scheduler mutex should never be locked when calling into +non-scheduler code, so it was converted to a non-recursive +std::mutex. = Lifecycle / thread-safety of Scheduler-based objects = @@ -407,9 +407,3 @@ will also affect the Gtk+ and KDE backend for the user event handling. This way it can be easier used to profile Tasks, eventually to improve LO's interactivity. - -== Convert SchedulerMutex to std::mutex == - -Since the SchedulerMutex is non-recursive, it could use a std::mutex instead -of simulating one still using the osl::Mutex. Not sure, if a deadlock instead -of a crash, when called recursively, is the better "user experience"... diff --git a/vcl/inc/schedulerimpl.hxx b/vcl/inc/schedulerimpl.hxx index 26a9c47de11f..f6d5dda708bd 100644 --- a/vcl/inc/schedulerimpl.hxx +++ b/vcl/inc/schedulerimpl.hxx @@ -21,7 +21,6 @@ #define INCLUDED_VCL_INC_SCHEDULERIMPL_HXX #include "salwtype.hxx" -#include <osl/mutex.hxx> #include <vcl/scheduler.hxx> class Task; @@ -38,22 +37,6 @@ struct ImplSchedulerData final const char *GetDebugName() const; }; -class SchedulerMutex final -{ - // this simulates a non-recursive mutex - bool m_bIsLocked; - osl::Mutex m_aMutex; - -public: - SchedulerMutex() - : m_bIsLocked(false) - { - } - - void acquire(); - void release(); -}; - class SchedulerGuard final { public: diff --git a/vcl/inc/svdata.hxx b/vcl/inc/svdata.hxx index e7a423234043..564c28bdc9b0 100644 --- a/vcl/inc/svdata.hxx +++ b/vcl/inc/svdata.hxx @@ -38,6 +38,7 @@ #include "salwtype.hxx" #include "displayconnectiondispatch.hxx" +#include <mutex> #include <vector> #include <unordered_map> #include <boost/functional/hash.hpp> @@ -369,7 +370,8 @@ struct ImplSchedulerContext SalTimer* mpSalTimer = nullptr; ///< interface to sal event loop / system timer sal_uInt64 mnTimerStart = 0; ///< start time of the timer sal_uInt64 mnTimerPeriod = SAL_MAX_UINT64; ///< current timer period - SchedulerMutex maMutex; ///< lock counting mutex for scheduler locking + std::mutex maMutex; ///< the "scheduler mutex" (see + ///< vcl/README.scheduler) bool mbActive = true; ///< is the scheduler active? }; diff --git a/vcl/source/app/scheduler.cxx b/vcl/source/app/scheduler.cxx index 1759178e2e2f..da8511bffa55 100644 --- a/vcl/source/app/scheduler.cxx +++ b/vcl/source/app/scheduler.cxx @@ -207,36 +207,18 @@ next_priority: rSchedCtx.mnTimerPeriod = InfiniteTimeoutMs; } -void SchedulerMutex::acquire() -{ - if (!m_aMutex.acquire()) - std::abort(); - if (m_bIsLocked) - std::abort(); - m_bIsLocked = true; -} - -void SchedulerMutex::release() -{ - if (!m_bIsLocked) - std::abort(); - m_bIsLocked = false; - if (!m_aMutex.release()) - std::abort(); -} - void Scheduler::Lock() { ImplSVData* pSVData = ImplGetSVData(); assert( pSVData != nullptr ); - pSVData->maSchedCtx.maMutex.acquire(); + pSVData->maSchedCtx.maMutex.lock(); } void Scheduler::Unlock() { ImplSVData* pSVData = ImplGetSVData(); assert( pSVData != nullptr ); - pSVData->maSchedCtx.maMutex.release(); + pSVData->maSchedCtx.maMutex.unlock(); } /** _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits