include/vcl/task.hxx | 9 +--------
vcl/inc/schedulerimpl.hxx | 1 +
vcl/opengl/gdiimpl.cxx | 3 +--
vcl/source/app/scheduler.cxx | 29 ++++++++++++++++++++++++++---
4 files changed, 29 insertions(+), 13 deletions(-)
New commits:
commit ffcff2fe24ce1fd64b7c45073c09f6d5a5a5d51d
Author: Jan-Marek Glogowski <glo...@fbihome.de>
AuthorDate: Wed Mar 13 01:23:36 2019 +0100
Commit: Jan-Marek Glogowski <glo...@fbihome.de>
CommitDate: Thu Mar 14 12:34:32 2019 +0100
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 <glo...@fbihome.de>
diff --git a/include/vcl/task.hxx b/include/vcl/task.hxx
index f5af890e2f57..9bc8b5c9bf2d 100644
--- a/include/vcl/task.hxx
+++ b/include/vcl/task.hxx
@@ -77,7 +77,7 @@ public:
virtual ~Task() COVERITY_NOEXCEPT_FALSE;
Task& operator=( const Task& rTask );
- inline void SetPriority(TaskPriority ePriority);
+ void SetPriority(TaskPriority ePriority);
TaskPriority GetPriority() const { return mePriority; }
void SetDebugName( const sal_Char *pDebugName ) { mpDebugName =
pDebugName; }
@@ -101,13 +101,6 @@ public:
bool IsStatic() const { return mbStatic; }
};
-inline void Task::SetPriority(TaskPriority ePriority)
-{
- SAL_WARN_IF(mpSchedulerData, "vcl.schedule",
- "Priority will just change after next schedule!");
- mePriority = ePriority;
-}
-
#endif // INCLUDED_VCL_TASK_HXX
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
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() )
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
