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

[Libreoffice Gerrit user]

 include/svl/listener.hxx        |    3 +++
 svl/source/notify/broadcast.cxx |   17 ++++++++++++-----
 svl/source/notify/listener.cxx  |   10 ++++++++++
 3 files changed, 25 insertions(+), 5 deletions(-)

New commits:
commit 2318e36d888ef0daa2e8d424fa7d3c2e423b5816
Author:     Noel Grandin <noelgrandin at gmail.com>
AuthorDate: Sat Sep 29 17:37:45 2018 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Mon Oct 1 08:15:16 2018 +0200

    scatter some asserts in SvtBroadcaster
    
    copying a broadcaster that has been marked for destruction is very suspicious.
    so is adding a listener to such a broadcaster
    
    Change-Id: Ic1cae111a600477f16a346004b8017a9a8d242e9
    Reviewed-on: https://gerrit.libreoffice.org/61136
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx
index 62a52dff18c5..d91a94610d8f 100644
--- a/svl/source/notify/broadcast.cxx
+++ b/svl/source/notify/broadcast.cxx
@@ -20,6 +20,7 @@
 #include <svl/broadcast.hxx>
 #include <svl/listener.hxx>
 #include <svl/hint.hxx>
+#include <cassert>
 #include <algorithm>
 
 void SvtBroadcaster::Normalize() const
@@ -43,6 +44,10 @@ void SvtBroadcaster::Normalize() const
 
 void SvtBroadcaster::Add( SvtListener* p )
 {
+    assert(!mbDisposing && "called inside my own destructor?");
+    assert(!mbAboutToDie && "called after PrepareForDestruction()?");
+    if (mbDisposing || mbAboutToDie)
+        return;
     maListeners.push_back(p);
     mbNormalized = false;
 }
@@ -76,6 +81,8 @@ SvtBroadcaster::SvtBroadcaster( const SvtBroadcaster &rBC ) :
     mbAboutToDie(rBC.mbAboutToDie), mbDisposing(false),
     mbNormalized(rBC.mbNormalized), mbDestNormalized(rBC.mbDestNormalized)
 {
+    assert(!mbAboutToDie && "copying an object marked with PrepareForDestruction()?");
+
     if (mbAboutToDie)
         Normalize();
 
commit 89e2ec08b50d88facd0b100a8be04ab56c1f3ad1
Author:     Noel Grandin <noelgrandin at gmail.com>
AuthorDate: Sat Sep 29 17:35:19 2018 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Mon Oct 1 08:15:07 2018 +0200

    don't call back into the SvtBroadcaster when dying
    
    and we don't need to use equal_range on a sorted and uniqued vector,
    lower_bound will do
    
    Change-Id: I3f967c04b43432875e3d4de75e6e87bfdef40dc8
    Reviewed-on: https://gerrit.libreoffice.org/61135
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/include/svl/listener.hxx b/include/svl/listener.hxx
index 5f2850ef7f19..d6dc7b0cddbb 100644
--- a/include/svl/listener.hxx
+++ b/include/svl/listener.hxx
@@ -28,10 +28,13 @@ class SfxHint;
 
 class SVL_DLLPUBLIC SvtListener
 {
+    friend class SvtBroadcaster;
     typedef std::unordered_set<SvtBroadcaster*> BroadcastersType;
     BroadcastersType maBroadcasters;
 
     const SvtListener&  operator=(const SvtListener &) = delete;
+    // called from the SvtBroadcaster destructor
+    void BroadcasterDying( SvtBroadcaster& rBroadcaster );
 
 public:
     class SVL_DLLPUBLIC QueryBase
diff --git a/svl/source/notify/broadcast.cxx b/svl/source/notify/broadcast.cxx
index 237c323e5d0b..62a52dff18c5 100644
--- a/svl/source/notify/broadcast.cxx
+++ b/svl/source/notify/broadcast.cxx
@@ -60,11 +60,11 @@ void SvtBroadcaster::Remove( SvtListener* p )
     }
 
     Normalize();
-    std::pair<ListenersType::iterator,ListenersType::iterator> r =
-        std::equal_range(maListeners.begin(), maListeners.end(), p);
 
-    if (r.first != r.second)
-        maListeners.erase(r.first, r.second);
+    auto it = std::lower_bound(maListeners.begin(), maListeners.end(), p);
+    if (it != maListeners.end() && *it == p)
+        maListeners.erase(it);
+
     if (maListeners.empty())
         ListenersGone();
 }
@@ -116,7 +116,7 @@ SvtBroadcaster::~SvtBroadcaster()
             ++dest;
 
         if (dest == maDestructedListeners.end() || *dest != *it)
-            (*it)->EndListening(*this);
+            (*it)->BroadcasterDying(*this);
     }
 }
 
diff --git a/svl/source/notify/listener.cxx b/svl/source/notify/listener.cxx
index 89306a534a0d..506647784451 100644
--- a/svl/source/notify/listener.cxx
+++ b/svl/source/notify/listener.cxx
@@ -19,6 +19,7 @@
 
 #include <svl/listener.hxx>
 #include <svl/broadcast.hxx>
+#include <cassert>
 
 SvtListener::QueryBase::QueryBase( sal_uInt16 nId ) : mnId(nId) {}
 SvtListener::QueryBase::~QueryBase() {}
@@ -65,6 +66,15 @@ bool SvtListener::EndListening( SvtBroadcaster& rBroadcaster )
     return true;
 }
 
+// called from the SvtBroadcaster destructor, used to avoid calling
+// back into the broadcaster again
+void SvtListener::BroadcasterDying( SvtBroadcaster& rBroadcaster )
+{
+    BroadcastersType::iterator it = maBroadcasters.find(&rBroadcaster);
+    if (it != maBroadcasters.end())
+        maBroadcasters.erase(it);
+}
+
 void SvtListener::EndListeningAll()
 {
     BroadcastersType::iterator it = maBroadcasters.begin();