sd: fix atexit crash in CanvasUpdateRequester::maRequesterMap

https://retrace.fedoraproject.org/faf/problems/bthash/?bth=2c9e4b335a4f17dea5e095f0a3e8f79aa40943cb

Clearly it's a really bad idea to own these objects at shutdown, so
instead use weak references, and also ensure that the
CanvasUpdateRequester is not deleted before the event is dispatched
just in case.

Change-Id: Iee8a109fe5969b2a3345e0ef266aee014962ae57

4 files changed, 53 insertions, 30 deletions

diff --git a/sd/source/ui/presenter/CanvasUpdateRequester.cxx b/sd/source/ui/presenter/CanvasUpdateRequester.cxx
index 90202e8138b4..ee385533773f 100644
--- a/sd/source/ui/presenter/CanvasUpdateRequester.cxx
+++ b/sd/source/ui/presenter/CanvasUpdateRequester.cxx
@@ -20,6 +20,7 @@
 #include "CanvasUpdateRequester.hxx"
 #include <vcl/svapp.hxx>
 #include <com/sun/star/lang/XComponent.hpp>
+#include <vector>
 
 using namespace ::com::sun::star;
 using namespace ::com::sun::star::uno;
@@ -36,30 +37,53 @@ public:
 
 //===== CanvasUpdateRequester =================================================
 
-CanvasUpdateRequester::RequesterMap CanvasUpdateRequester::maRequesterMap;
-
 std::shared_ptr<CanvasUpdateRequester> CanvasUpdateRequester::Instance (
     const Reference<rendering::XSpriteCanvas>& rxSharedCanvas)
 {
-    RequesterMap::const_iterator iRequester;
-    for (iRequester=maRequesterMap.begin(); iRequester!=maRequesterMap.end(); ++iRequester)
+    // this global must not own anything or we crash on shutdown
+    static std::vector<std::pair<
+        uno::WeakReference<rendering::XSpriteCanvas>,
+        std::weak_ptr<CanvasUpdateRequester>>> s_RequesterMap;
+    for (auto it = s_RequesterMap.begin(); it != s_RequesterMap.end(); ++it)
     {
-        if (iRequester->first == rxSharedCanvas)
-            return iRequester->second;
+        uno::Reference<rendering::XSpriteCanvas> const xCanvas(it->first);
+        if (xCanvas == rxSharedCanvas)
+        {
+            std::shared_ptr<CanvasUpdateRequester> pRequester(it->second);
+            if (pRequester)
+            {
+                return pRequester;
+            }
+            else
+            {
+                std::shared_ptr<CanvasUpdateRequester> const pNew(
+                        new CanvasUpdateRequester(rxSharedCanvas), Deleter());
+                it->second = pNew;
+                return pNew;
+            }
+        }
+        else
+        {
+            if (!xCanvas.is())
+            {
+                it = s_RequesterMap.erase(it); // remove stale entry
+            }
+        }
     }
 
     // No requester for the given canvas found.  Create a new one.
     std::shared_ptr<CanvasUpdateRequester> pRequester (
         new CanvasUpdateRequester(rxSharedCanvas), Deleter());
-    maRequesterMap.push_back(RequesterMap::value_type(rxSharedCanvas,pRequester));
+    s_RequesterMap.push_back(std::make_pair(rxSharedCanvas, pRequester));
     return pRequester;
 }
 
+
 CanvasUpdateRequester::CanvasUpdateRequester (
-    const Reference<rendering::XSpriteCanvas>& rxCanvas)
-    : mxCanvas(rxCanvas),
-      mnUserEventId(nullptr),
-      mbUpdateFlag(false)
+        const Reference<rendering::XSpriteCanvas>& rxCanvas)
+    : mxCanvas(rxCanvas)
+    , m_pUserEventId(nullptr)
+    , mbUpdateFlag(false)
 {
     Reference<lang::XComponent> xComponent (mxCanvas, UNO_QUERY);
     if (xComponent.is())
@@ -70,16 +94,16 @@ CanvasUpdateRequester::CanvasUpdateRequester (
 
 CanvasUpdateRequester::~CanvasUpdateRequester()
 {
-    if (mnUserEventId != nullptr)
-        Application::RemoveUserEvent(mnUserEventId);
+    assert(m_pUserEventId == nullptr);
 }
 
 void CanvasUpdateRequester::RequestUpdate (const bool bUpdateAll)
 {
-    if (mnUserEventId == nullptr)
+    if (m_pUserEventId == nullptr)
     {
+        m_pThis = shared_from_this(); // keep instance alive until dispatch
         mbUpdateFlag = bUpdateAll;
-        mnUserEventId = Application::PostUserEvent(LINK(this, CanvasUpdateRequester, Callback));
+        m_pUserEventId = Application::PostUserEvent(LINK(this, CanvasUpdateRequester, Callback));
     }
     else
     {
@@ -89,12 +113,14 @@ void CanvasUpdateRequester::RequestUpdate (const bool bUpdateAll)
 
 IMPL_LINK_NOARG(CanvasUpdateRequester, Callback, void*, void)
 {
-    mnUserEventId = nullptr;
+    m_pUserEventId = nullptr;
     if (mxCanvas.is())
     {
         mxCanvas->updateScreen(mbUpdateFlag);
         mbUpdateFlag = false;
     }
+    assert(m_pThis);
+    m_pThis.reset(); // possibly delete "this"
 }
 
 } } // end of namespace ::sd::presenter
diff --git a/sd/source/ui/presenter/CanvasUpdateRequester.hxx b/sd/source/ui/presenter/CanvasUpdateRequester.hxx
index a712cc502200..471b9ab53b72 100644
--- a/sd/source/ui/presenter/CanvasUpdateRequester.hxx
+++ b/sd/source/ui/presenter/CanvasUpdateRequester.hxx
@@ -22,10 +22,8 @@
 
 #include <com/sun/star/rendering/XSpriteCanvas.hpp>
 #include <sal/types.h>
-#include <tools/solar.h>
 #include <tools/link.hxx>
 #include <memory>
-#include <vector>
 
 struct ImplSVEvent;
 
@@ -37,6 +35,7 @@ namespace sd { namespace presenter {
     to a single call to updateScreen.
 */
 class CanvasUpdateRequester
+    : public std::enable_shared_from_this<CanvasUpdateRequester>
 {
 public:
     CanvasUpdateRequester(const CanvasUpdateRequester&) = delete;
@@ -55,15 +54,12 @@ private:
     ~CanvasUpdateRequester();
     class Deleter; friend class Deleter;
 
-    typedef ::std::vector<
-        ::std::pair<
-            css::uno::Reference<css::rendering::XSpriteCanvas>,
-           std::shared_ptr<CanvasUpdateRequester> > > RequesterMap;
-    static RequesterMap maRequesterMap;
-
+    /// keep instance alive waiting for event dispatch
+    std::shared_ptr<CanvasUpdateRequester> m_pThis;
     css::uno::Reference<css::rendering::XSpriteCanvas> mxCanvas;
-    ImplSVEvent * mnUserEventId;
+    ImplSVEvent * m_pUserEventId;
     bool mbUpdateFlag;
+
     DECL_LINK(Callback, void*, void);
 };
 
diff --git a/sd/source/ui/presenter/PresenterCanvas.cxx b/sd/source/ui/presenter/PresenterCanvas.cxx
index e9905cae8db5..7acab0c60f43 100644
--- a/sd/source/ui/presenter/PresenterCanvas.cxx
+++ b/sd/source/ui/presenter/PresenterCanvas.cxx
@@ -109,7 +109,6 @@ PresenterCanvas::PresenterCanvas (
       mxSharedWindow(rxSharedWindow),
       mxWindow(rxWindow),
       maOffset(),
-      mpUpdateRequester(),
       maClipRectangle(),
       mbOffsetUpdatePending(true)
 {
@@ -117,7 +116,9 @@ PresenterCanvas::PresenterCanvas (
         mxWindow->addWindowListener(this);
 
     if (mxUpdateCanvas.is())
-        mpUpdateRequester = CanvasUpdateRequester::Instance(mxUpdateCanvas);
+    {
+        m_pUpdateRequester = CanvasUpdateRequester::Instance(mxUpdateCanvas);
+    }
 }
 
 PresenterCanvas::~PresenterCanvas()
@@ -414,9 +415,9 @@ sal_Bool SAL_CALL PresenterCanvas::updateScreen (sal_Bool bUpdateAll)
     ThrowIfDisposed();
 
     mbOffsetUpdatePending = true;
-    if (mpUpdateRequester.get() != nullptr)
+    if (m_pUpdateRequester.get() != nullptr)
     {
-        mpUpdateRequester->RequestUpdate(bUpdateAll);
+        m_pUpdateRequester->RequestUpdate(bUpdateAll);
         return true;
     }
     else
diff --git a/sd/source/ui/presenter/PresenterCanvas.hxx b/sd/source/ui/presenter/PresenterCanvas.hxx
index 52163258bce1..b9d91932871e 100644
--- a/sd/source/ui/presenter/PresenterCanvas.hxx
+++ b/sd/source/ui/presenter/PresenterCanvas.hxx
@@ -300,7 +300,7 @@ private:
     /** The UpdateRequester is used by updateScreen() to schedule
         updateScreen() calls at the shared canvas.
     */
-    std::shared_ptr<CanvasUpdateRequester> mpUpdateRequester;
+    std::shared_ptr<CanvasUpdateRequester> m_pUpdateRequester;
 
     /** The clip rectangle as given to SetClip().
     */