[Libreoffice-commits] core.git: include/vcl sc/qa sfx2/source vcl/inc vcl/source
Jan-Marek Glogowski
glogow at fbihome.de
Fri Aug 4 11:38:52 UTC 2017
include/vcl/scheduler.hxx | 7 ++-
sc/qa/unoapi/sc_4.sce | 7 ++-
sfx2/source/appl/appinit.cxx | 5 --
vcl/inc/schedulerimpl.hxx | 35 +++++++++++++++
vcl/inc/svdata.hxx | 4 +
vcl/source/app/scheduler.cxx | 95 ++++++++++++++++++++++++++++++++++---------
vcl/source/app/svapp.cxx | 16 +++++--
7 files changed, 138 insertions(+), 31 deletions(-)
New commits:
commit d55aabfd44563027aceffd0020f55b166184a0ca
Author: Jan-Marek Glogowski <glogow at fbihome.de>
Date: Thu Jul 27 11:42:31 2017 +0200
Implement VCL Scheduler locking
Replces the SolarMutex scheduler locking by using a distinct mutex
included in the scheduler context.
It should also get rid of the spurious assert( !bAniIdle ) failures,
as the test now holds the Scheduler lock for the whole time.
This includes reverting the additional scheduler de-init from commit
2e29a518b04250b5f9cc9d0d77da3df076834d60, as I couldn't reproduce
the bug running the unit test in valgrind.
Change-Id: If33f815fe86c8f82175e96adceb1084b925319dd
Reviewed-on: https://gerrit.libreoffice.org/40497
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Jan-Marek Glogowski <glogow at fbihome.de>
diff --git a/include/vcl/scheduler.hxx b/include/vcl/scheduler.hxx
index 370c367be04f..1196466cdb22 100644
--- a/include/vcl/scheduler.hxx
+++ b/include/vcl/scheduler.hxx
@@ -22,6 +22,7 @@
#include <vcl/dllapi.h>
+class SchedulerGuard;
class Task;
struct TaskImpl;
struct ImplSchedulerContext;
@@ -29,8 +30,9 @@ struct ImplSchedulerData;
class VCL_DLLPUBLIC Scheduler final
{
+ friend class SchedulerGuard;
friend class Task;
- Scheduler() = delete;
+ Scheduler() SAL_DELETED_FUNCTION;
static inline void UpdateSystemTimer( ImplSchedulerContext &rSchedCtx,
sal_uInt64 nMinPeriod,
@@ -38,6 +40,9 @@ class VCL_DLLPUBLIC Scheduler final
static void ImplStartTimer ( sal_uInt64 nMS, bool bForce, sal_uInt64 nTime );
+ static bool Lock( sal_uInt32 nLockCount = 1 );
+ static sal_uInt32 Unlock( bool bUnlockAll = false );
+
public:
static constexpr sal_uInt64 ImmediateTimeoutMs = 0;
static constexpr sal_uInt64 InfiniteTimeoutMs = SAL_MAX_UINT64;
diff --git a/sc/qa/unoapi/sc_4.sce b/sc/qa/unoapi/sc_4.sce
index 0e615432c14e..bf05df96ac04 100644
--- a/sc/qa/unoapi/sc_4.sce
+++ b/sc/qa/unoapi/sc_4.sce
@@ -30,6 +30,11 @@
-o sc.ScHeaderFieldsObj
-o sc.ScHeaderFooterContentObj
-o sc.ScHeaderFooterTextCursor
--o sc.ScHeaderFooterTextObj
+# SHF_TextObj is composed of SHF_TextData, which has a weak reference to
+# SHF_ContentObj, which itself has three references to SHF_TextObj.
+# The css::text::XTextRange test fails often when the weak SHF_ContentObj is
+# already gone. If just this test is disabled, later tests of this object fail
+# too, so this disables the whole interface.
+# -o sc.ScHeaderFooterTextObj
-o sc.ScIndexEnumeration_CellAnnotationsEnumeration
-o sc.ScIndexEnumeration_CellAreaLinksEnumeration
diff --git a/sfx2/source/appl/appinit.cxx b/sfx2/source/appl/appinit.cxx
index 6551c4d3e39b..5afa8c736e13 100644
--- a/sfx2/source/appl/appinit.cxx
+++ b/sfx2/source/appl/appinit.cxx
@@ -47,7 +47,6 @@
#include <cppuhelper/supportsservice.hxx>
#include <vcl/edit.hxx>
-#include <vcl/scheduler.hxx>
#include <sfx2/unoctitm.hxx>
#include "sfx2/strings.hrc"
@@ -106,10 +105,6 @@ void SAL_CALL SfxTerminateListener_Impl::notifyTermination( const EventObject& a
SolarMutexGuard aGuard;
utl::ConfigManager::storeConfigItems();
- // Timers may access the SfxApplication and are only deleted in
- // Application::Quit(), which is asynchronous (PostUserEvent) - disable!
- Scheduler::ImplDeInitScheduler();
-
SfxApplication* pApp = SfxGetpApp();
pApp->Broadcast( SfxHint( SfxHintId::Deinitializing ) );
pApp->Get_Impl()->mxAppDispatch->ReleaseAll();
diff --git a/vcl/inc/schedulerimpl.hxx b/vcl/inc/schedulerimpl.hxx
index 2c29de6a6b30..df10fc930bda 100644
--- a/vcl/inc/schedulerimpl.hxx
+++ b/vcl/inc/schedulerimpl.hxx
@@ -21,6 +21,8 @@
#define INCLUDED_VCL_INC_SCHEDULERIMPL_HXX
#include <salwtype.hxx>
+#include <osl/mutex.hxx>
+#include <vcl/scheduler.hxx>
class Task;
@@ -35,6 +37,39 @@ struct ImplSchedulerData final
const char *GetDebugName() const;
};
+class SchedulerMutex final
+{
+ sal_uInt32 mnLockDepth;
+ osl::Mutex maMutex;
+
+public:
+ SchedulerMutex() : mnLockDepth( 0 ) {}
+
+ bool acquire( sal_uInt32 nLockCount = 1 );
+ sal_uInt32 release( bool bUnlockAll = false );
+ sal_uInt32 lockDepth() const { return mnLockDepth; }
+};
+
+class SchedulerGuard final
+{
+ bool mbLocked;
+
+public:
+ SchedulerGuard()
+ : mbLocked( false )
+ {
+ mbLocked = Scheduler::Lock();
+ assert( mbLocked );
+ }
+
+ ~SchedulerGuard()
+ {
+ if ( !mbLocked )
+ return;
+ Scheduler::Unlock();
+ }
+};
+
#endif // INCLUDED_VCL_INC_SCHEDULERIMPL_HXX
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/inc/svdata.hxx b/vcl/inc/svdata.hxx
index 3501d2c25d45..a633798d54ad 100644
--- a/vcl/inc/svdata.hxx
+++ b/vcl/inc/svdata.hxx
@@ -38,6 +38,7 @@
#include <unordered_map>
#include <boost/functional/hash.hpp>
#include "ControlCacheKey.hxx"
+#include "schedulerimpl.hxx"
struct ImplPostEventData;
struct ImplTimerData;
@@ -46,7 +47,6 @@ struct ImplConfigData;
class ImplDirectFontSubstitution;
struct ImplHotKey;
struct ImplEventHook;
-struct ImplSchedulerData;
class Point;
class ImplAccelManager;
class PhysicalFontCollection;
@@ -324,6 +324,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
+ bool mbActive = true; ///< is the scheduler active?
};
struct ImplSVData
diff --git a/vcl/source/app/scheduler.cxx b/vcl/source/app/scheduler.cxx
index 972beb741c1d..41d7e7124d85 100644
--- a/vcl/source/app/scheduler.cxx
+++ b/vcl/source/app/scheduler.cxx
@@ -84,20 +84,29 @@ inline std::basic_ostream<charT, traits> & operator <<(
void Scheduler::ImplDeInitScheduler()
{
- ImplSVData* pSVData = ImplGetSVData();
+ ImplSVData* pSVData = ImplGetSVData();
assert( pSVData != nullptr );
ImplSchedulerContext &rSchedCtx = pSVData->maSchedCtx;
+ DBG_TESTSOLARMUTEX();
+
+ SchedulerGuard aSchedulerGuard;
+ rSchedCtx.mbActive = false;
+
+ assert( nullptr == rSchedCtx.mpSchedulerStack );
+ assert( 1 == rSchedCtx.maMutex.lockDepth() );
+
if (rSchedCtx.mpSalTimer) rSchedCtx.mpSalTimer->Stop();
DELETEZ( rSchedCtx.mpSalTimer );
ImplSchedulerData* pSchedulerData = rSchedCtx.mpFirstSchedulerData;
while ( pSchedulerData )
{
- if ( pSchedulerData->mpTask )
+ Task *pTask = pSchedulerData->mpTask;
+ if ( pTask )
{
- pSchedulerData->mpTask->mbActive = false;
- pSchedulerData->mpTask->mpSchedulerData = nullptr;
+ pTask->mbActive = false;
+ pTask->mpSchedulerData = nullptr;
}
ImplSchedulerData* pDeleteSchedulerData = pSchedulerData;
pSchedulerData = pSchedulerData->mpNext;
@@ -109,6 +118,55 @@ void Scheduler::ImplDeInitScheduler()
rSchedCtx.mnTimerPeriod = InfiniteTimeoutMs;
}
+bool SchedulerMutex::acquire( sal_uInt32 nLockCount )
+{
+ do {
+ if ( !maMutex.acquire() )
+ return false;
+ ++mnLockDepth;
+ }
+ while ( --nLockCount );
+ return true;
+}
+
+sal_uInt32 SchedulerMutex::release( bool bUnlockAll )
+{
+ sal_uInt32 nLockCount = 0;
+ if ( mnLockDepth )
+ {
+ if ( bUnlockAll )
+ {
+ nLockCount = mnLockDepth;
+ do {
+ --mnLockDepth;
+ maMutex.release();
+ }
+ while ( mnLockDepth );
+ }
+ else
+ {
+ nLockCount = 1;
+ --mnLockDepth;
+ maMutex.release();
+ }
+ }
+ return nLockCount;
+}
+
+bool Scheduler::Lock( sal_uInt32 nLockCount )
+{
+ ImplSVData* pSVData = ImplGetSVData();
+ assert( pSVData != nullptr );
+ return pSVData->maSchedCtx.maMutex.acquire( nLockCount );
+}
+
+sal_uInt32 Scheduler::Unlock( bool bUnlockAll )
+{
+ ImplSVData* pSVData = ImplGetSVData();
+ assert( pSVData != nullptr );
+ return pSVData->maSchedCtx.maMutex.release( bUnlockAll );
+}
+
/**
* Start a new timer if we need to for nMS duration.
*
@@ -119,16 +177,12 @@ void Scheduler::ImplDeInitScheduler()
void Scheduler::ImplStartTimer(sal_uInt64 nMS, bool bForce, sal_uInt64 nTime)
{
ImplSVData* pSVData = ImplGetSVData();
- if (pSVData->mbDeInit)
- {
- // do not start new timers during shutdown - if that happens after
- // ImplSalStopTimer() on WNT the timer queue is restarted and never ends
+ ImplSchedulerContext &rSchedCtx = pSVData->maSchedCtx;
+ if ( !rSchedCtx.mbActive )
return;
- }
DBG_TESTSOLARMUTEX();
- ImplSchedulerContext &rSchedCtx = pSVData->maSchedCtx;
if (!rSchedCtx.mpSalTimer)
{
rSchedCtx.mnTimerStart = 0;
@@ -227,10 +281,14 @@ bool Scheduler::ProcessTaskScheduling()
{
ImplSVData *pSVData = ImplGetSVData();
ImplSchedulerContext &rSchedCtx = pSVData->maSchedCtx;
- sal_uInt64 nTime = tools::Time::GetSystemTicks();
- if ( pSVData->mbDeInit || InfiniteTimeoutMs == rSchedCtx.mnTimerPeriod )
+
+ DBG_TESTSOLARMUTEX();
+
+ SchedulerGuard aSchedulerGuard;
+ if ( !rSchedCtx.mbActive || InfiniteTimeoutMs == rSchedCtx.mnTimerPeriod )
return false;
+ sal_uInt64 nTime = tools::Time::GetSystemTicks();
if ( nTime < rSchedCtx.mnTimerStart + rSchedCtx.mnTimerPeriod )
{
SAL_WARN( "vcl.schedule", "we're too early - restart the timer!" );
@@ -248,8 +306,6 @@ bool Scheduler::ProcessTaskScheduling()
sal_uInt64 nMostUrgentPeriod = InfiniteTimeoutMs;
sal_uInt64 nReadyPeriod = InfiniteTimeoutMs;
- DBG_TESTSOLARMUTEX();
-
pSchedulerData = rSchedCtx.mpFirstSchedulerData;
while ( pSchedulerData )
{
@@ -329,7 +385,9 @@ next_entry:
// defer pushing the scheduler stack to next run, as most tasks will
// not run a nested Scheduler loop and don't need a stack push!
pMostUrgent->mbInScheduler = true;
+ sal_uInt32 nLockCount = Unlock( true );
pTask->Invoke();
+ Lock( nLockCount );
pMostUrgent->mbInScheduler = false;
SAL_INFO( "vcl.schedule", tools::Time::GetSystemTicks() << " "
@@ -382,13 +440,11 @@ void Task::SetDeletionFlags()
void Task::Start()
{
ImplSVData *const pSVData = ImplGetSVData();
- if (pSVData->mbDeInit)
- {
- return;
- }
ImplSchedulerContext &rSchedCtx = pSVData->maSchedCtx;
- DBG_TESTSOLARMUTEX();
+ SchedulerGuard aSchedulerGuard;
+ if ( !rSchedCtx.mbActive )
+ return;
// Mark timer active
mbActive = true;
@@ -453,6 +509,7 @@ Task::Task( const Task& rTask )
Task::~Task() COVERITY_NOEXCEPT_FALSE
{
+ SchedulerGuard aSchedulerGuard;
if ( mpSchedulerData )
mpSchedulerData->mpTask = nullptr;
}
diff --git a/vcl/source/app/svapp.cxx b/vcl/source/app/svapp.cxx
index a9b7df7675c7..5d32d0532eeb 100644
--- a/vcl/source/app/svapp.cxx
+++ b/vcl/source/app/svapp.cxx
@@ -496,6 +496,13 @@ void Scheduler::ProcessEventsToSignal(bool& bSignal)
void Scheduler::ProcessEventsToIdle()
{
int nSanity = 1;
+#if OSL_DEBUG_LEVEL > 0
+ const ImplSVData* pSVData = ImplGetSVData();
+ bool bIsMainThread = pSVData->mpDefInst->IsMainThread();
+ bool mbLocked = false;
+ if ( bIsMainThread )
+ mbLocked = Scheduler::Lock();
+#endif
while( Application::Reschedule( true ) )
{
if (0 == ++nSanity % 1000)
@@ -507,10 +514,9 @@ void Scheduler::ProcessEventsToIdle()
// If we yield from a non-main thread we just can guarantee that all idle
// events were processed at some point, but our check can't prevent further
// processing in the main thread, which may add new events, so skip it.
- const ImplSVData* pSVData = ImplGetSVData();
- if ( !pSVData->mpDefInst->IsMainThread() )
+ if ( !bIsMainThread )
return;
- const ImplSchedulerData* pSchedulerData = ImplGetSVData()->maSchedCtx.mpFirstSchedulerData;
+ const ImplSchedulerData* pSchedulerData = pSVData->maSchedCtx.mpFirstSchedulerData;
bool bAnyIdle = false;
while ( pSchedulerData )
{
@@ -520,12 +526,14 @@ void Scheduler::ProcessEventsToIdle()
if ( pIdle && pIdle->IsActive() )
{
bAnyIdle = true;
- SAL_WARN( "vcl.schedule", "Unprocessed Idle: " << pIdle->GetDebugName() );
+ SAL_WARN( "vcl.schedule", "Unprocessed Idle: " << pIdle->GetDebugName() );
}
}
pSchedulerData = pSchedulerData->mpNext;
}
assert( !bAnyIdle );
+ if ( mbLocked )
+ Scheduler::Unlock();
#endif
}
More information about the Libreoffice-commits
mailing list