tdf#58624 sw: fix ~SwAccessibleContext() use-after-free race

As seen in JunitTest_toolkit_unoapi_1:

Method doAccessibleAction() finished with state OK
LOG> doAccessibleAction(): COMPLETED.OK
debug:27272:12:  -SwAccessibleParagraph mutexwait 0x3fd9f50
debug:27272:9:  SwAccessibleContext::Dispose 0x3872620 11SwRootFrame
debug:27272:9:  SwAccessibleContext::DisposeChildren 0x4047c80 0x386d600 11SwPageFrame
debug:27272:9:  SwAccessibleContext::DisposeChildren xAcc 0
debug:27272:9:  SwAccessibleContext::DisposeChildren 0x4047c80 0x386cef0 11SwBodyFrame
debug:27272:9:  SwAccessibleContext::DisposeChildren xAcc 0
debug:27272:9:  SwAccessibleContext::DisposeChildren 0x4047c80 0x3878fe0 11SwTextFrame
debug:27272:9:  SwAccessibleContext::DisposeChildren xAcc 0
debug:27272:9:  SwAccessibleMap::RemoveContext erase 0x3872620
debug:27272:9:  ~SwAccessibleMap: frame entry 0x3878fe0
debug:27272:9:  ~SwAccessibleMap: mpFrameMap 0x3eb64a0
debug:27272:9:  ~SwAccessibleMap: mpShapeMap 0
soffice.bin: sw/source/core/access/accmap.cxx:1726: virtual SwAccessibleMap::~SwAccessibleMap(): Assertion `(!mpFrameMap || mpFrameMap->empty()) && "Frame map should be empty after disposing the root frame"'

The problem here is that thread 12 is blocked on SolarMutex in
~SwAccessibleParagraph(), while thread 9 is in ~SwAccessibleMap().

This means that in SwAccessibleContext::DisposeChildren(), the
WeakReference to the SwAccessibleParagraph cannot create a
uno::Reference because its reference count is 0, so
SwAccessibleContext::Dispose() is not called on it and it remains
in the SwAccessibleMap::mpFrameMap.

This triggers the assert and later on ~SwAccessibleContext() would
access the deleted SwAccessibleMap and crash.

To fix this, introduce a weak reference from SwAccessibleContext to
SwAccessibleMap; use a std::weak_ptr because that is not derived from
OWeakObject.

The weak_ptr is only used in the dtor ~SwAccessibleContext(); as
long as the ref-count of SwAccessibleContext is > 0 it is guaranteed
that the SwAccessibleContext::m_pMap is either null or valid
as the recursive Dispose() will work fine.

It is possible that additional temporary owning references could delay
the destruction of SwAccessibleMap, and the order of destruction
of Writer documents is very fragile, so rely on the SolarMutex lock
to prevent that; the only shared_ptr that owns SwAccessibleMap while
SolarMutex is not locked is the one in SwViewShellImp.

(An alternative fix would be to represent the 3 lifecycle stages of
SwAccessibleContext by adding a C++-pointer to the
SwAccessibleMap::mpFrameMap, so that DisposeChildren() can, if
the WeakReference is no longer valid due to ref-count 0, use the
pointer and clear SwAccessibleContext::m_pMap - this and the
corresponding call to SwAccessibleMap::RemoveContext() from
~SwAccessibleContext() under a mutex that is shared_ptr-owned by
SwAccessibleMap and all SwAccessibleContext.)

Change-Id: If2b44c79189e3b3d276491a5c57d5404bb2be71a

1 files changed, 4 insertions, 1 deletions

diff --git a/sw/inc/accmap.hxx b/sw/inc/accmap.hxx
index 9d155dcb2edb..d59852b671c2 100644
--- a/sw/inc/accmap.hxx
+++ b/sw/inc/accmap.hxx
@@ -30,10 +30,12 @@
 #include <svx/AccessibleControlShape.hxx>
 #include <svx/AccessibleShape.hxx>
 #include "fesh.hxx"
+#include <o3tl/typed_flags_set.hxx>
+
 #include <list>
 #include <vector>
+#include <memory>
 #include <set>
-#include <o3tl/typed_flags_set.hxx>
 
 class SwAccessibleParagraph;
 class SwViewShell;
@@ -88,6 +90,7 @@ namespace o3tl
 
 class SwAccessibleMap : public ::accessibility::IAccessibleViewForwarder,
                         public ::accessibility::IAccessibleParent
+                , public std::enable_shared_from_this<SwAccessibleMap>
 {
     mutable ::osl::Mutex maMutex;
     ::osl::Mutex maEventMutex;