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.

(cherry picked from commit baa253d00ab6aa67fa100472db00227baa14c87a)

sd: oops, skipped one entry on every erase by incrementing
Thanks Noel for the hint.
(cherry picked from commit 670631cae260263e4822d8aa63bc5c2e4349c796)

Change-Id: Iee8a109fe5969b2a3345e0ef266aee014962ae57
Reviewed-on: https://gerrit.libreoffice.org/37263
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Thorsten Behrens <Thorsten.Behrens@CIB.de>

4 files changed, 54 insertions, 30 deletions

diff --git a/sd/source/ui/presenter/CanvasUpdateRequester.cxx b/sd/source/ui/presenter/CanvasUpdateRequester.cxx
index 90202e8138b4..456c37d43a58 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,54 @@ 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(); )
     {
-        if (iRequester->first == rxSharedCanvas)
-            return iRequester->second;
+        uno::Reference<rendering::XSpriteCanvas> const xCanvas(it->first);
+        if (!xCanvas.is())
+        {
+            it = s_RequesterMap.erase(it); // remove stale entry
+        }
+        else 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
+        {
+            ++it;
+        }
     }
 
     // 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 +95,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 +114,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 6333a536123e..0bd4ecc43d18 100644
--- a/sd/source/ui/presenter/PresenterCanvas.cxx
+++ b/sd/source/ui/presenter/PresenterCanvas.cxx
@@ -119,7 +119,6 @@ PresenterCanvas::PresenterCanvas (
       mxSharedWindow(rxSharedWindow),
       mxWindow(rxWindow),
       maOffset(),
-      mpUpdateRequester(),
       maClipRectangle(),
       mbOffsetUpdatePending(true)
 {
@@ -127,7 +126,9 @@ PresenterCanvas::PresenterCanvas (
         mxWindow->addWindowListener(this);
 
     if (mxUpdateCanvas.is())
-        mpUpdateRequester = CanvasUpdateRequester::Instance(mxUpdateCanvas);
+    {
+        m_pUpdateRequester = CanvasUpdateRequester::Instance(mxUpdateCanvas);
+    }
 }
 
 PresenterCanvas::~PresenterCanvas()
@@ -465,9 +466,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 a31e899d0234..4c5784251734 100644
--- a/sd/source/ui/presenter/PresenterCanvas.hxx
+++ b/sd/source/ui/presenter/PresenterCanvas.hxx
@@ -353,7 +353,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().
     */