[Libreoffice-commits] core.git: include/svl svl/source

[Libreoffice Gerrit user]

 include/svl/broadcast.hxx       |    1 
 svl/source/notify/broadcast.cxx |   74 +++++++++++++++++-----------------------
 2 files changed, 33 insertions(+), 42 deletions(-)

New commits:
commit 97dac306e6b4c26f67307e6b3bb0c2b2b470a09a
Author:     Noel Grandin <noelgrandin at gmail.com>
AuthorDate: Sat Oct 6 12:31:22 2018 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Sat Oct 6 13:27:42 2018 +0200

    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 at collabora.co.uk>

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() {}