Kaydet (Commit) 2601708e authored tarafından Noel Grandin's avatar Noel Grandin

cleanup mutex and signalling in ExecuteWrapper

found this while examining issues reported by clang-tidy.

Judging from the code, the original intention was to somehow lock access
to the mbSignal field in ExecuteWrapper, but since we were not locking
when writing to that field, and that field was not volatile, it
surprises me that this worked very well at all.

We can accomplish the same end more reliably by just marking the field
as volatile and not doing any locking at all.

Change-Id: I388c7082a809b6aca5a3c8981625f55cfef3cfcd
Reviewed-on: https://gerrit.libreoffice.org/60184
Tested-by: Jenkins
Reviewed-by: 's avatarNoel Grandin <noel.grandin@collabora.co.uk>
üst 40069adb
......@@ -58,8 +58,7 @@ class ExecuteWrapper
{
std::function<void()> mFunc;
Link<Timer*, void> mHandler;
bool mbSignal;
std::mutex mMutex;
volatile bool mbSignal;
public:
......@@ -75,11 +74,6 @@ public:
mbSignal = true;
}
std::mutex& getMutex()
{
return mMutex;
}
DECL_LINK( ExecuteActionHdl, Timer*, void );
};
......@@ -96,21 +90,9 @@ IMPL_LINK_NOARG(ExecuteWrapper, ExecuteActionHdl, Timer*, void)
aIdle.Start();
}
for (;;) {
{
std::unique_lock<std::mutex> lock(mMutex);
if (mbSignal) {
break;
}
}
while (!mbSignal) {
Application::Reschedule();
}
std::unique_lock<std::mutex> lock(mMutex);
while (!mbSignal)
{
// coverity[sleep] - intentional sleep while mutex held
std::this_thread::sleep_for(std::chrono::milliseconds(5));
}
}
delete this;
}
......@@ -146,7 +128,6 @@ void SAL_CALL UIObjectUnoObj::executeAction(const OUString& rAction, const css::
};
ExecuteWrapper* pWrapper = new ExecuteWrapper(func, LINK(this, UIObjectUnoObj, NotifyHdl));
std::unique_lock<std::mutex>(pWrapper->getMutex());
aIdle->SetInvokeHandler(LINK(pWrapper, ExecuteWrapper, ExecuteActionHdl));
{
SolarMutexGuard aGuard;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment