From 414172d7fafc2d2c6b1e698aeb67fdb5797b118d Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Sun, 19 Feb 2023 15:28:06 +0200 Subject: 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 --- include/comphelper/interfacecontainer4.hxx | 25 +++++++++++++++++++------ include/comphelper/multiinterfacecontainer4.hxx | 22 +++++++++++++++++++--- 2 files changed, 38 insertions(+), 9 deletions(-) (limited to 'include') 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& /*rGuard*/, + OInterfaceIteratorHelper4(std::unique_lock& rGuard, OInterfaceContainerHelper4& 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 inline void OInterfaceContainerHelper4::forEach(std::unique_lock& rGuard, FuncT const& func) const { + assert(rGuard.owns_lock()); if (std::as_const(maData)->size() == 0) { return; @@ -321,23 +324,29 @@ inline void OInterfaceContainerHelper4::notifyEach( template sal_Int32 -OInterfaceContainerHelper4::getLength(std::unique_lock& /*rGuard*/) const +OInterfaceContainerHelper4::getLength(std::unique_lock& rGuard) const { + assert(rGuard.owns_lock()); + (void)rGuard; return maData->size(); } template std::vector> -OInterfaceContainerHelper4::getElements(std::unique_lock& /*rGuard*/) const +OInterfaceContainerHelper4::getElements(std::unique_lock& rGuard) const { + assert(rGuard.owns_lock()); + (void)rGuard; return *maData; } template sal_Int32 -OInterfaceContainerHelper4::addInterface(std::unique_lock& /*rGuard*/, +OInterfaceContainerHelper4::addInterface(std::unique_lock& rGuard, const css::uno::Reference& rListener) { + assert(rGuard.owns_lock()); + (void)rGuard; assert(rListener.is()); maData->push_back(rListener); return maData->size(); @@ -345,8 +354,10 @@ OInterfaceContainerHelper4::addInterface(std::unique_lock template sal_Int32 OInterfaceContainerHelper4::removeInterface( - std::unique_lock& /*rGuard*/, const css::uno::Reference& rListener) + std::unique_lock& rGuard, const css::uno::Reference& 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::disposeAndClear(std::unique_lock -void OInterfaceContainerHelper4::clear(::std::unique_lock<::std::mutex>& /*rGuard*/) +void OInterfaceContainerHelper4::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 getContainedTypes(std::unique_lock& rGuard) const { + assert(rGuard.owns_lock()); std::vector aInterfaceTypes; aInterfaceTypes.reserve(m_aMap.size()); for (const auto& rPair : m_aMap) @@ -56,6 +57,7 @@ public: } inline bool hasContainedTypes(std::unique_lock& 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& 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 aIt(rGuard, *rPair.second); + OInterfaceIteratorHelper4 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& /*rGuard*/) + inline void clear(std::unique_lock& rGuard) { + assert(rGuard.owns_lock()); + (void)rGuard; for (const auto& rPair : m_aMap) rPair.second->clear(); } @@ -170,9 +184,11 @@ private: typedef ::std::vector>>> InterfaceMap; InterfaceMap m_aMap; - typename InterfaceMap::const_iterator find(std::unique_lock& /*rGuard*/, + typename InterfaceMap::const_iterator find(std::unique_lock& rGuard, const key& rKey) const { + assert(rGuard.owns_lock()); + (void)rGuard; auto iter = m_aMap.begin(); auto end = m_aMap.end(); while (iter != end) -- cgit stro/mimo/mimo-7-6'>distro/mimo/mimo-7-6 LibreOffice 核心代码仓库文档基金会
summaryrefslogtreecommitdiff