[Libreoffice-commits] core.git: vcl/source

[Libreoffice Gerrit user]

 vcl/source/uitest/uno/uiobject_uno.cxx |   23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

New commits:
commit 2601708e3c00092693af6dd04561125cafb21d8e
Author:     Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Sat Sep 8 09:44:33 2018 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Tue Sep 11 19:07:58 2018 +0200

    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: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/vcl/source/uitest/uno/uiobject_uno.cxx b/vcl/source/uitest/uno/uiobject_uno.cxx
index a121889e67c9..f95a67747e25 100644
--- a/vcl/source/uitest/uno/uiobject_uno.cxx
+++ b/vcl/source/uitest/uno/uiobject_uno.cxx
@@ -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;