[Libreoffice-commits] core.git: Branch 'libreoffice-5-3' - sd/source
Michael Stahl
mstahl at redhat.com
Fri May 12 11:57:04 UTC 2017
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 <mstahl at 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 <ci at libreoffice.org>
Reviewed-by: Thorsten Behrens <Thorsten.Behrens at 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().
*/
More information about the Libreoffice-commits
mailing list