fix locking in OMultiTypeInterfaceContainerHelperVar4::diposeAndCloear

where I forgot to relock at the end of the method

And sprinkle some asserts around to catch mistakes like this in future

Change-Id: I4908ac0ffdfe33b1b5cf1b02e6765f20973af841
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147296
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 38 insertions, 9 deletions

diff --git a/include/comphelper/interfacecontainer4.hxx b/include/comphelper/interfacecontainer4.hxx
index 3a6696fda8c5..bfa3a3059212 100644
--- a/include/comphelper/interfacecontainer4.hxx
+++ b/include/comphelper/interfacecontainer4.hxx
@@ -60,13 +60,15 @@ public:
        @param rGuard
             this parameter only here to make that this container is accessed while locked
      */
-    OInterfaceIteratorHelper4(std::unique_lock<std::mutex>& /*rGuard*/,
+    OInterfaceIteratorHelper4(std::unique_lock<std::mutex>& rGuard,
                               OInterfaceContainerHelper4<ListenerT>& rCont_)
         : rCont(rCont_)
         , maData(rCont.maData)
         // const_cast so we don't trigger make_unique via o3tl::cow_wrapper::operator->
         , nRemain(std::as_const(maData)->size())
     {
+        assert(rGuard.owns_lock());
+        (void)rGuard;
     }
 
     /** Return true, if there are more elements in the iterator. */
@@ -281,6 +283,7 @@ template <typename FuncT>
 inline void OInterfaceContainerHelper4<T>::forEach(std::unique_lock<std::mutex>& rGuard,
                                                    FuncT const& func) const
 {
+    assert(rGuard.owns_lock());
     if (std::as_const(maData)->size() == 0)
     {
         return;
@@ -321,23 +324,29 @@ inline void OInterfaceContainerHelper4<ListenerT>::notifyEach(
 
 template <class ListenerT>
 sal_Int32
-OInterfaceContainerHelper4<ListenerT>::getLength(std::unique_lock<std::mutex>& /*rGuard*/) const
+OInterfaceContainerHelper4<ListenerT>::getLength(std::unique_lock<std::mutex>& rGuard) const
 {
+    assert(rGuard.owns_lock());
+    (void)rGuard;
     return maData->size();
 }
 
 template <class ListenerT>
 std::vector<css::uno::Reference<ListenerT>>
-OInterfaceContainerHelper4<ListenerT>::getElements(std::unique_lock<std::mutex>& /*rGuard*/) const
+OInterfaceContainerHelper4<ListenerT>::getElements(std::unique_lock<std::mutex>& rGuard) const
 {
+    assert(rGuard.owns_lock());
+    (void)rGuard;
     return *maData;
 }
 
 template <class ListenerT>
 sal_Int32
-OInterfaceContainerHelper4<ListenerT>::addInterface(std::unique_lock<std::mutex>& /*rGuard*/,
+OInterfaceContainerHelper4<ListenerT>::addInterface(std::unique_lock<std::mutex>& rGuard,
                                                     const css::uno::Reference<ListenerT>& rListener)
 {
+    assert(rGuard.owns_lock());
+    (void)rGuard;
     assert(rListener.is());
     maData->push_back(rListener);
     return maData->size();
@@ -345,8 +354,10 @@ OInterfaceContainerHelper4<ListenerT>::addInterface(std::unique_lock<std::mutex>
 
 template <class ListenerT>
 sal_Int32 OInterfaceContainerHelper4<ListenerT>::removeInterface(
-    std::unique_lock<std::mutex>& /*rGuard*/, const css::uno::Reference<ListenerT>& rListener)
+    std::unique_lock<std::mutex>& rGuard, const css::uno::Reference<ListenerT>& rListener)
 {
+    assert(rGuard.owns_lock());
+    (void)rGuard;
     assert(rListener.is());
 
     // It is not valid to compare the pointer directly, but it's faster.
@@ -395,8 +406,10 @@ void OInterfaceContainerHelper4<ListenerT>::disposeAndClear(std::unique_lock<std
 }
 
 template <class ListenerT>
-void OInterfaceContainerHelper4<ListenerT>::clear(::std::unique_lock<::std::mutex>& /*rGuard*/)
+void OInterfaceContainerHelper4<ListenerT>::clear(::std::unique_lock<::std::mutex>& rGuard)
 {
+    assert(rGuard.owns_lock());
+    (void)rGuard;
     maData->clear();
 }
 }
diff --git a/include/comphelper/multiinterfacecontainer4.hxx b/include/comphelper/multiinterfacecontainer4.hxx
index 1241951f5505..2212d638410d 100644
--- a/include/comphelper/multiinterfacecontainer4.hxx
+++ b/include/comphelper/multiinterfacecontainer4.hxx
@@ -45,6 +45,7 @@ public:
      */
     inline std::vector<key> getContainedTypes(std::unique_lock<std::mutex>& rGuard) const
     {
+        assert(rGuard.owns_lock());
         std::vector<key> aInterfaceTypes;
         aInterfaceTypes.reserve(m_aMap.size());
         for (const auto& rPair : m_aMap)
@@ -56,6 +57,7 @@ public:
     }
     inline bool hasContainedTypes(std::unique_lock<std::mutex>& rGuard) const
     {
+        assert(rGuard.owns_lock());
         for (const auto& rPair : m_aMap)
             // are interfaces added to this container?
             if (rPair.second->getLength(rGuard))
@@ -133,15 +135,24 @@ public:
     inline void disposeAndClear(std::unique_lock<std::mutex>& rGuard,
                                 const css::lang::EventObject& rEvt)
     {
+        assert(rGuard.owns_lock());
         // create a copy, because do not fire event in a guarded section
         InterfaceMap tempMap;
         {
             tempMap = std::move(m_aMap);
         }
         rGuard.unlock();
+        // So... we don't want to hold the normal mutex while we fire
+        // the events, but the calling convention here wants a mutex, so
+        // just create a temporary/fake one. Since the listeners we
+        // are working with are now function-local, we don't really need
+        // a mutex at all, but it's easier to create a fake one than
+        // create a bunch of special-case code for this situation.
+        std::mutex tempMutex;
+        std::unique_lock tempGuard(tempMutex);
         for (auto& rPair : tempMap)
         {
-            OInterfaceIteratorHelper4<listener> aIt(rGuard, *rPair.second);
+            OInterfaceIteratorHelper4<listener> aIt(tempGuard, *rPair.second);
             while (aIt.hasMoreElements())
             {
                 try
@@ -155,12 +166,15 @@ public:
                 }
             }
         }
+        rGuard.lock(); // return with lock in same state as entry
     }
     /**
       Remove all elements of all containers. Does not delete the container.
      */
-    inline void clear(std::unique_lock<std::mutex>& /*rGuard*/)
+    inline void clear(std::unique_lock<std::mutex>& rGuard)
     {
+        assert(rGuard.owns_lock());
+        (void)rGuard;
         for (const auto& rPair : m_aMap)
             rPair.second->clear();
     }
@@ -170,9 +184,11 @@ private:
     typedef ::std::vector<std::pair<key, std::unique_ptr<OInterfaceContainerHelper4<listener>>>>
         InterfaceMap;
     InterfaceMap m_aMap;
-    typename InterfaceMap::const_iterator find(std::unique_lock<std::mutex>& /*rGuard*/,
+    typename InterfaceMap::const_iterator find(std::unique_lock<std::mutex>& rGuard,
                                                const key& rKey) const
     {
+        assert(rGuard.owns_lock());
+        (void)rGuard;
         auto iter = m_aMap.begin();
         auto end = m_aMap.end();
         while (iter != end)