sd/source/ui/presenter/CanvasUpdateRequester.cxx | 59 ++++++++++++++++------- sd/source/ui/presenter/CanvasUpdateRequester.hxx | 14 +---- sd/source/ui/presenter/PresenterCanvas.cxx | 9 +-- sd/source/ui/presenter/PresenterCanvas.hxx | 2 4 files changed, 54 insertions(+), 30 deletions(-)
New commits: commit 0f54a0f218834bfcc74b00f0a105785d3099bd40 Author: Michael Stahl <mst...@redhat.com> Date: Thu May 4 16:17:16 2017 +0200 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 <c...@libreoffice.org> Reviewed-by: Thorsten Behrens <thorsten.behr...@cib.de> 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(). */ _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits