sw: fix crash on deleting a bookmark with active selection listeners

The document had a registered selection listener that created UNO
cursors (bookmarks) in its selectionChanged() callback. Once an UNO API
user tried to delete a bookmark, Writer crashed.

What happens is that sw::mark::MarkManager::deleteMark() created an
iterator pointing to an element of m_vAllMarks, then called
sw::mark::Bookmark::DeregisterFromDoc(), which indirectly calls
SwXTextView::NotifySelChanged(), which invokes UNO selection listeners,
which are allowed to create UNO cursors, which modify m_vAllMarks,
invalidating the iterator.

Fix the problem by first creating a non-const iterator based on ppMark,
then conditionally invoke DeregisterFromDoc(). If DeregisterFromDoc() is
called, then update the iterator based on pMark, so it is valid by the
time we erase the bookmark from m_vAllMarks.

This was not the problem when erasing the same mark from m_vBookmarks,
as in that case the iterator creation and the erase() both happen before
the de-registration.

Change-Id: Iaae95ec9c3038e8ee3b84408094844d0ff678213
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141489
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

1 files changed, 18 insertions, 3 deletions

diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index d38542922b52..c19d182c3f7a 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -1285,13 +1285,28 @@ namespace sw::mark
                 // no special marks container
                 break;
         }
-        DdeBookmark* const pDdeBookmark = dynamic_cast<DdeBookmark*>(pMark);
-        if (pDdeBookmark)
-            pDdeBookmark->DeregisterFromDoc(m_rDoc);
         //Effective STL Item 27, get a non-const iterator aI at the same
         //position as const iterator ppMark was
         auto aI = m_vAllMarks.begin();
         std::advance(aI, std::distance<container_t::const_iterator>(aI, ppMark.get()));
+        DdeBookmark* const pDdeBookmark = dynamic_cast<DdeBookmark*>(pMark);
+        if (pDdeBookmark)
+        {
+            pDdeBookmark->DeregisterFromDoc(m_rDoc);
+
+            // Update aI, possibly a selection listener invalidated the iterators of m_vAllMarks.
+            auto [it, endIt] = equal_range(m_vAllMarks.begin(), m_vAllMarks.end(),
+                                           pMark->GetMarkStart(), CompareIMarkStartsBefore());
+            for (; it != endIt; ++it)
+            {
+                if (*it == pMark)
+                {
+                    aI = m_vAllMarks.begin();
+                    std::advance(aI, std::distance<container_t::const_iterator>(aI, it));
+                    break;
+                }
+            }
+        }
 
         m_vAllMarks.erase(aI);
         // If we don't have a lazy deleter