diff options
author | Jan-Marek Glogowski <glogow@fbihome.de> | 2019-03-13 01:23:36 +0100 |
---|---|---|
committer | Jan-Marek Glogowski <glogow@fbihome.de> | 2019-03-14 12:34:32 +0100 |
commit | ffcff2fe24ce1fd64b7c45073c09f6d5a5a5d51d (patch) | |
tree | 53cef736da5e1830f6207495ec6ead3e5ec17720 /vcl | |
parent | 8bc23968129fc38d70fb614668a411599d4e6d0e (diff) |
Fix scheduled Task priority change handling
If a task is still in the scheduler priority queue and its
priority is changed, it won't be moved into the correct queue.
We have to track the priority of the scheduled task, so we can
warn the developer to fix the code and actually handle re-start
correctly.
Since we don't want to traverse the whole Scheduler queues on
priority change (which sometimes get very long) to remove the
wrong data item, we'll just invalidate it, if a priority change
is detected.
This also reverts commit d24b264c4a47 ("vcl opengl: avoid task
priority warning on cursor blink"), which tried to avoid the
warning, which was just half right and independent of the broken
priority change handling.
LO doesn't change priorities of scheduled tasks normally, so
that bug didn't turn out to have much impact, I guess.
Change-Id: I6e46b518a7c3532047c619c013bd8597f73ed7a6
Reviewed-on: https://gerrit.libreoffice.org/69249
Tested-by: Jenkins
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>
Diffstat (limited to 'vcl')
-rw-r--r-- | vcl/inc/schedulerimpl.hxx | 1 | ||||
-rw-r--r-- | vcl/opengl/gdiimpl.cxx | 3 | ||||
-rw-r--r-- | vcl/source/app/scheduler.cxx | 29 |
3 files changed, 28 insertions, 5 deletions
diff --git a/vcl/inc/schedulerimpl.hxx b/vcl/inc/schedulerimpl.hxx index 6517809634ae..38f27a665186 100644 --- a/vcl/inc/schedulerimpl.hxx +++ b/vcl/inc/schedulerimpl.hxx @@ -33,6 +33,7 @@ struct ImplSchedulerData final Task* mpTask; ///< Pointer to VCL Task instance bool mbInScheduler; ///< Task currently processed? sal_uInt64 mnUpdateTime; ///< Last Update Time + TaskPriority mePriority; ///< Task priority const char *GetDebugName() const; }; diff --git a/vcl/opengl/gdiimpl.cxx b/vcl/opengl/gdiimpl.cxx index caecec0d4621..90922d737eb3 100644 --- a/vcl/opengl/gdiimpl.cxx +++ b/vcl/opengl/gdiimpl.cxx @@ -63,9 +63,8 @@ public: virtual void Invoke() override { m_pImpl->doFlush(); - if (GetPriority() != TaskPriority::HIGHEST) - SetPriority(TaskPriority::HIGHEST); Stop(); + SetPriority(TaskPriority::HIGHEST); } }; diff --git a/vcl/source/app/scheduler.cxx b/vcl/source/app/scheduler.cxx index 39fe10322902..c2a3b07d4d3a 100644 --- a/vcl/source/app/scheduler.cxx +++ b/vcl/source/app/scheduler.cxx @@ -316,7 +316,10 @@ static void AppendSchedulerData( ImplSchedulerContext &rSchedCtx, ImplSchedulerData * const pSchedulerData) { assert(pSchedulerData->mpTask); - const int nTaskPriority = static_cast<int>(pSchedulerData->mpTask->GetPriority()); + pSchedulerData->mePriority = pSchedulerData->mpTask->GetPriority(); + pSchedulerData->mpNext = nullptr; + + const int nTaskPriority = static_cast<int>(pSchedulerData->mePriority); if (!rSchedCtx.mpLastSchedulerData[nTaskPriority]) { rSchedCtx.mpFirstSchedulerData[nTaskPriority] = pSchedulerData; @@ -327,7 +330,6 @@ static void AppendSchedulerData( ImplSchedulerContext &rSchedCtx, rSchedCtx.mpLastSchedulerData[nTaskPriority]->mpNext = pSchedulerData; rSchedCtx.mpLastSchedulerData[nTaskPriority] = pSchedulerData; } - pSchedulerData->mpNext = nullptr; } static ImplSchedulerData* DropSchedulerData( @@ -555,7 +557,17 @@ void Task::Start() if ( !rSchedCtx.mbActive ) return; - // Mark timer active + // is the task scheduled in the correct priority queue? + // if not we have to get a new data object, as we don't want to traverse + // the whole list to move the data to the correct list, as the task list + // is just single linked. + // Task priority doesn't change that often AFAIK, or we might need to + // start caching ImplSchedulerData objects. + if (mpSchedulerData && mpSchedulerData->mePriority != mePriority) + { + mpSchedulerData->mpTask = nullptr; + mpSchedulerData = nullptr; + } mbActive = true; if ( !mpSchedulerData ) @@ -564,6 +576,7 @@ void Task::Start() ImplSchedulerData* pSchedulerData = new ImplSchedulerData; pSchedulerData->mpTask = this; pSchedulerData->mbInScheduler = false; + // mePriority is set in AppendSchedulerData mpSchedulerData = pSchedulerData; AppendSchedulerData( rSchedCtx, pSchedulerData ); @@ -584,6 +597,16 @@ void Task::Stop() mbActive = false; } +void Task::SetPriority(TaskPriority ePriority) +{ + // you don't actually need to call Stop() before but Start() after, but we + // can't check that and don't know when Start() should be called. + SAL_WARN_IF(mpSchedulerData && mbActive, "vcl.schedule", + "Stop the task before changing the priority, as it will just " + "change after the task was scheduled with the old prio!"); + mePriority = ePriority; +} + Task& Task::operator=( const Task& rTask ) { if ( IsActive() ) |