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

[Caolán McNamara (via logerrit)]

 vcl/inc/graphic/Manager.hxx    |    2 ++
 vcl/source/graphic/Manager.cxx |   15 +++++++++++++++
 2 files changed, 17 insertions(+)

New commits:
commit 5198a68d7e8e4fe8397fa188c19ba8be587e29f5
Author:     Caolán McNamara <caolanm at redhat.com>
AuthorDate: Thu Apr 4 17:43:57 2019 +0100
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Sat Apr 6 16:42:48 2019 +0200

    crashtesting, threading crash with bitmapinfoaccess_null_ptr_deref.sample
    
    on export to .ods
    
    Thread 1:
    
    5  0x00007f758fde4969 in raise () from /lib64/libc.so.6
    6  0x00007f758fde5f98 in abort () from /lib64/libc.so.6
    7  0x00007f758f485afd in __gnu_debug::_Error_formatter::_M_error (
       this=0x7f7588872860 <__gnu_debug::_Error_formatter::_M_at(char const*, unsigned int)::__formatter>)
       at ../../../.././libstdc++-v3/src/c++11/debug.cc:1069
    8  0x00007f7587b42922 in __gnu_debug::_Safe_iterator<std::__detail::_Node_iterator<ImpGraphic*, true, false>, std::__debug::unordered_set<ImpGraphic*, std::hash<ImpGraphic*>, std::equal_to<ImpGraphic*>, std::allocator<ImpGraphic*> > >::operator++ (
       this=0x7f755a58e440) at /srv/crashtestdata/gccbuild/gcc8/include/c++/8.2.0/debug/safe_iterator.h:295
    9  0x00007f7587b415d5 in vcl::graphic::Manager::registerGraphic (this=0x5fe9860,
       pImpGraphic=std::shared_ptr<ImpGraphic> (use count 1, weak count 0) = {...})
       at /home/buildslave/source/libo-core/vcl/source/graphic/Manager.cxx:123
    10 0x00007f7587b418e2 in vcl::graphic::Manager::newInstance (this=0x5fe9860)
       at /home/buildslave/source/libo-core/vcl/source/graphic/Manager.cxx:156
    11 0x00007f758798c406 in Graphic::Graphic (this=0x7f755a58e6d0)
       at /home/buildslave/source/libo-core/vcl/source/gdi/graph.cxx:187
    12 0x00007f758799c85a in ImpGraphic::loadPrepared (this=0x80d2500)
       at /home/buildslave/source/libo-core/vcl/source/gdi/impgraph.cxx:1540
    13 0x00007f758799c999 in ImpGraphic::ImplSwapIn (this=0x80d2500)
       at /home/buildslave/source/libo-core/vcl/source/gdi/impgraph.cxx:1561
    14 0x00007f758799c813 in ImpGraphic::ensureAvailable (this=0x80d2500)
    
    and a bunch of other threads looking like they've just come from the same
    family of methods
    
    Change-Id: Ic13d3c7cb2fb4adaa2a0a6b8845fc2156d53005e
    Reviewed-on: https://gerrit.libreoffice.org/70271
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/vcl/inc/graphic/Manager.hxx b/vcl/inc/graphic/Manager.hxx
index f6f24b47db6f..1f897ecf7146 100644
--- a/vcl/inc/graphic/Manager.hxx
+++ b/vcl/inc/graphic/Manager.hxx
@@ -23,6 +23,7 @@
 #include <unotools/configmgr.hxx>
 
 #include <memory>
+#include <mutex>
 #include <chrono>
 #include <unordered_set>
 
@@ -35,6 +36,7 @@ namespace graphic
 class Manager final
 {
 private:
+    std::recursive_mutex maMutex; // instead of SolarMutex because graphics can live past vcl main
     std::unordered_set<ImpGraphic*> m_pImpGraphicList;
     std::chrono::seconds mnAllowedIdleTime;
     sal_Int64 mnMemoryLimit;
diff --git a/vcl/source/graphic/Manager.cxx b/vcl/source/graphic/Manager.cxx
index 405aa8928649..fe80fdf5aaa4 100644
--- a/vcl/source/graphic/Manager.cxx
+++ b/vcl/source/graphic/Manager.cxx
@@ -20,6 +20,7 @@
 #include <graphic/Manager.hxx>
 #include <impgraph.hxx>
 #include <vcl/lazydelete.hxx>
+#include <vcl/svapp.hxx>
 #include <sal/log.hxx>
 
 using namespace css;
@@ -72,6 +73,8 @@ Manager::Manager()
 
 void Manager::reduceGraphicMemory()
 {
+    std::lock_guard<std::recursive_mutex> aGuard(maMutex);
+
     for (ImpGraphic* pEachImpGraphic : m_pImpGraphicList)
     {
         if (mnUsedSize < mnMemoryLimit * 0.7)
@@ -102,6 +105,8 @@ sal_Int64 Manager::getGraphicSizeBytes(const ImpGraphic* pImpGraphic)
 
 IMPL_LINK(Manager, SwapOutTimerHandler, Timer*, pTimer, void)
 {
+    std::lock_guard<std::recursive_mutex> aGuard(maMutex);
+
     pTimer->Stop();
     reduceGraphicMemory();
     pTimer->Start();
@@ -110,6 +115,8 @@ IMPL_LINK(Manager, SwapOutTimerHandler, Timer*, pTimer, void)
 void Manager::registerGraphic(const std::shared_ptr<ImpGraphic>& pImpGraphic,
                               OUString const& /*rsContext*/)
 {
+    std::lock_guard<std::recursive_mutex> aGuard(maMutex);
+
     // make some space first
     if (mnUsedSize > mnMemoryLimit)
         reduceGraphicMemory();
@@ -139,6 +146,8 @@ void Manager::registerGraphic(const std::shared_ptr<ImpGraphic>& pImpGraphic,
 
 void Manager::unregisterGraphic(ImpGraphic* pImpGraphic)
 {
+    std::lock_guard<std::recursive_mutex> aGuard(maMutex);
+
     mnUsedSize -= getGraphicSizeBytes(pImpGraphic);
     m_pImpGraphicList.erase(pImpGraphic);
 }
@@ -201,16 +210,22 @@ std::shared_ptr<ImpGraphic> Manager::newInstance(const GraphicExternalLink& rGra
 
 void Manager::swappedIn(const ImpGraphic* pImpGraphic)
 {
+    std::lock_guard<std::recursive_mutex> aGuard(maMutex);
+
     mnUsedSize += getGraphicSizeBytes(pImpGraphic);
 }
 
 void Manager::swappedOut(const ImpGraphic* pImpGraphic)
 {
+    std::lock_guard<std::recursive_mutex> aGuard(maMutex);
+
     mnUsedSize -= getGraphicSizeBytes(pImpGraphic);
 }
 
 void Manager::changeExisting(const ImpGraphic* pImpGraphic, sal_Int64 nOldSizeBytes)
 {
+    std::lock_guard<std::recursive_mutex> aGuard(maMutex);
+
     mnUsedSize -= nOldSizeBytes;
     mnUsedSize += getGraphicSizeBytes(pImpGraphic);
 }