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@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

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;

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);
 }