try to fix SvtBroadcaster on macOS

this reverts

commit a20ec3b5cd691ae18f0d87d045673ce3a06579c6
fix iterator invalidation assert
and
commit 4a290888580474f9542f185091bb2a6fcf4e9a53
SvtBroadcaster unify the normal and PrepareForDestruction paths

no idea what is going on, I suspect that std::vector is re-allocating
its data buffer despite having called reserve()

Change-Id: I681ae5fca4578240a2a0dc0af51ff06d919f9099
Reviewed-on: https://gerrit.libreoffice.org/61464
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 33 insertions, 42 deletions

diff --git a/include/svl/broadcast.hxx b/include/svl/broadcast.hxx
index 27181e03294e..b4c8be5f9ac0 100644
--- a/include/svl/broadcast.hxx
+++ b/include/svl/broadcast.hxx
@@ -82,6 +82,7 @@ private:
 
     /// Indicate that this broadcaster will be destructed (we indicate this on all ScColumn's broadcasters during the ScTable destruction, eg.)
     bool mbAboutToDie:1;
+    bool mbDisposing:1;
     mutable bool mbNormalized:1;
     mutable bool mbDestNormalized:1;
 };
diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx
index bef049a84c5c..626a48a332e9 100644
--- a/svl/source/notify/broadcast.cxx
+++ b/svl/source/notify/broadcast.cxx
@@ -40,8 +40,9 @@ void SvtBroadcaster::Normalize() const
 
 void SvtBroadcaster::Add( SvtListener* p )
 {
-    assert(!mbAboutToDie && "called inside my own destructor / after PrepareForDestruction()?");
-    if (mbAboutToDie)
+    assert(!mbDisposing && "called inside my own destructor?");
+    assert(!mbAboutToDie && "called after PrepareForDestruction()?");
+    if (mbDisposing || mbAboutToDie)
         return;
     // only reset mbNormalized if we are going to become unsorted
     if (!maListeners.empty() && maListeners.back() > p)
@@ -51,12 +52,13 @@ void SvtBroadcaster::Add( SvtListener* p )
 
 void SvtBroadcaster::Remove( SvtListener* p )
 {
+    if (mbDisposing)
+        return;
+
     if (mbAboutToDie)
     {
-        // only reset mbDestNormalized if we are going to become unsorted
-        if (!maDestructedListeners.empty() && maDestructedListeners.back() > p)
-            mbDestNormalized = false;
         maDestructedListeners.push_back(p);
+        mbDestNormalized = false;
         return;
     }
 
@@ -73,13 +75,14 @@ void SvtBroadcaster::Remove( SvtListener* p )
         ListenersGone();
 }
 
-SvtBroadcaster::SvtBroadcaster() : mbAboutToDie(false), mbNormalized(true), mbDestNormalized(true) {}
+SvtBroadcaster::SvtBroadcaster() : mbAboutToDie(false), mbDisposing(false), mbNormalized(true), mbDestNormalized(true) {}
 
 SvtBroadcaster::SvtBroadcaster( const SvtBroadcaster &rBC ) :
-    mbAboutToDie(false),
+    mbAboutToDie(false), mbDisposing(false),
     mbNormalized(true), mbDestNormalized(true)
 {
-    assert(!rBC.mbAboutToDie && "copying an object marked with PrepareForDestruction() / that is in it's destructor?");
+    assert(!rBC.mbAboutToDie && "copying an object marked with PrepareForDestruction()?");
+    assert(!rBC.mbDisposing && "copying an object that is in it's destructor?");
 
     rBC.Normalize(); // so that insert into ourself is in-order, and therefore we do not need to Normalize()
     maListeners.reserve(rBC.maListeners.size());
@@ -89,54 +92,41 @@ SvtBroadcaster::SvtBroadcaster( const SvtBroadcaster &rBC ) :
 
 SvtBroadcaster::~SvtBroadcaster()
 {
-    PrepareForDestruction();
-
-    Normalize();
-
-    {
-        auto dest(maDestructedListeners.data());
-        SfxHint aHint(SfxHintId::Dying);
-        for (ListenersType::iterator it(maListeners.begin()); it != maListeners.end(); ++it)
-        {
-            auto destEnd(dest + maDestructedListeners.size());
-            // skip the destructed ones
-            while (dest != destEnd && (*dest < *it))
-                ++dest;
-
-            if (dest == destEnd || *dest != *it)
-                (*it)->Notify(aHint);
-        }
-    }
+    mbDisposing = true;
+    Broadcast( SfxHint(SfxHintId::Dying) );
 
     Normalize();
 
     // now when both lists are sorted, we can linearly unregister all
     // listeners, with the exception of those that already asked to be removed
     // during their own destruction
+    ListenersType::const_iterator dest(maDestructedListeners.begin());
+    for (ListenersType::iterator it(maListeners.begin()); it != maListeners.end(); ++it)
     {
-        auto dest(maDestructedListeners.data());
-        for (ListenersType::iterator it(maListeners.begin()); it != maListeners.end(); ++it)
-        {
-            auto destEnd(dest + maDestructedListeners.size());
-            // skip the destructed ones
-            while (dest != destEnd && (*dest < *it))
-                ++dest;
-
-            if (dest == destEnd || *dest != *it)
-                (*it)->BroadcasterDying(*this);
-        }
+        // skip the destructed ones
+        while (dest != maDestructedListeners.end() && (*dest < *it))
+            ++dest;
+
+        if (dest == maDestructedListeners.end() || *dest != *it)
+            (*it)->BroadcasterDying(*this);
     }
 }
 
 void SvtBroadcaster::Broadcast( const SfxHint &rHint )
 {
-    assert(!mbAboutToDie && "broadcasting after PrepareForDestruction() / from inside my own destructor?");
-    if (mbAboutToDie)
-        return;
+    Normalize();
 
+    ListenersType::const_iterator dest(maDestructedListeners.begin());
     ListenersType aListeners(maListeners); // this copy is important to avoid erasing entries while iterating
-    for (SvtListener * p : aListeners)
-        p->Notify(rHint);
+    for (ListenersType::iterator it(aListeners.begin()); it != aListeners.end(); ++it)
+    {
+        // skip the destructed ones
+        while (dest != maDestructedListeners.end() && (*dest < *it))
+            ++dest;
+
+        if (dest == maDestructedListeners.end() || *dest != *it)
+            (*it)->Notify(rHint);
+    }
 }
 
 void SvtBroadcaster::ListenersGone() {}