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

[Noel Grandin (via logerrit)]

 vcl/inc/graphic/Manager.hxx    |    4 ++--
 vcl/source/graphic/Manager.cxx |   23 ++++++++++++++---------
 2 files changed, 16 insertions(+), 11 deletions(-)

New commits:
commit 9df17d12a0e069d0a0db262368abc153b92169a0
Author:     Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Wed Sep 22 12:29:15 2021 +0200
Commit:     Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Wed Sep 22 14:57:36 2021 +0200

    fix deadlock in vcl::GraphicManager
    
    after
        commit 300753bf1d4db7eff42d707f427180f0d1d1dffb
        no need to use recursive_mutex in graphic::Manager
    
    Change-Id: I85b6f83d513ea1998e1bd7c3be5cea999c590c5b
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122426
    Tested-by: Jenkins
    Tested-by: Noel Grandin <noel.gran...@collabora.co.uk>
    Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>

diff --git a/vcl/inc/graphic/Manager.hxx b/vcl/inc/graphic/Manager.hxx
index 60dc62ab184c..6324ff89a899 100644
--- a/vcl/inc/graphic/Manager.hxx
+++ b/vcl/inc/graphic/Manager.hxx
@@ -43,11 +43,12 @@ private:
     Manager();
 
     void registerGraphic(const std::shared_ptr<ImpGraphic>& rImpGraphic);
-    void loopGraphicsAndSwapOut();
+    void loopGraphicsAndSwapOut(std::unique_lock<std::mutex>& rGuard);
 
     DECL_LINK(SwapOutTimerHandler, Timer*, void);
 
     static sal_Int64 getGraphicSizeBytes(const ImpGraphic* pImpGraphic);
+    void reduceGraphicMemory(std::unique_lock<std::mutex>& rGuard);
 
 public:
     static Manager& get();
@@ -55,7 +56,6 @@ public:
     void swappedIn(const ImpGraphic* pImpGraphic, sal_Int64 nSizeBytes);
     void swappedOut(const ImpGraphic* pImpGraphic, sal_Int64 nSizeBytes);
 
-    void reduceGraphicMemory();
     void changeExisting(const ImpGraphic* pImpGraphic, sal_Int64 nOldSize);
     void unregisterGraphic(ImpGraphic* pImpGraphic);
 
diff --git a/vcl/source/graphic/Manager.cxx b/vcl/source/graphic/Manager.cxx
index 92293dc99db6..e637e16e1442 100644
--- a/vcl/source/graphic/Manager.cxx
+++ b/vcl/source/graphic/Manager.cxx
@@ -76,7 +76,7 @@ Manager::Manager()
     }
 }
 
-void Manager::loopGraphicsAndSwapOut()
+void Manager::loopGraphicsAndSwapOut(std::unique_lock<std::mutex>& rGuard)
 {
     // make a copy of m_pImpGraphicList because if we swap out a svg, the svg
     // filter may create more temp Graphics which are auto-added to
@@ -102,28 +102,33 @@ void Manager::loopGraphicsAndSwapOut()
                 auto aSeconds = 
std::chrono::duration_cast<std::chrono::seconds>(aDeltaTime);
 
                 if (aSeconds > mnAllowedIdleTime)
+                {
+                    // unlock because svgio can call back into us
+                    rGuard.unlock();
                     pEachImpGraphic->swapOut();
+                    rGuard.lock();
+                }
             }
         }
     }
 }
 
-void Manager::reduceGraphicMemory()
+void Manager::reduceGraphicMemory(std::unique_lock<std::mutex>& rGuard)
 {
+    // maMutex is locked in callers
+
     if (!mbSwapEnabled)
         return;
 
     if (mnUsedSize < mnMemoryLimit)
         return;
 
-    std::scoped_lock aGuard(maMutex);
-
     // avoid recursive reduceGraphicMemory on reexport of tdf118346-1.odg to 
odg
     if (mbReducingGraphicMemory)
         return;
     mbReducingGraphicMemory = true;
 
-    loopGraphicsAndSwapOut();
+    loopGraphicsAndSwapOut(rGuard);
 
     sal_Int64 calculatedSize = 0;
     for (ImpGraphic* pEachImpGraphic : m_pImpGraphicList)
@@ -151,20 +156,20 @@ sal_Int64 Manager::getGraphicSizeBytes(const ImpGraphic* 
pImpGraphic)
 
 IMPL_LINK(Manager, SwapOutTimerHandler, Timer*, pTimer, void)
 {
-    std::scoped_lock aGuard(maMutex);
+    std::unique_lock aGuard(maMutex);
 
     pTimer->Stop();
-    reduceGraphicMemory();
+    reduceGraphicMemory(aGuard);
     pTimer->Start();
 }
 
 void Manager::registerGraphic(const std::shared_ptr<ImpGraphic>& pImpGraphic)
 {
-    std::scoped_lock aGuard(maMutex);
+    std::unique_lock aGuard(maMutex);
 
     // make some space first
     if (mnUsedSize > mnMemoryLimit)
-        reduceGraphicMemory();
+        reduceGraphicMemory(aGuard);
 
     // Insert and update the used size (bytes)
     mnUsedSize += getGraphicSizeBytes(pImpGraphic.get());