From b788e6e8fd4debbb38a03c374929b6af4678d7e5 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Sun, 22 Aug 2021 14:59:52 +0200 Subject: osl::Mutex->std::mutex in AccessibleEventNotifier which means we need a new interface container to handle std::mutex. Take the opportunity to move most of the locking outside the InterfaceContainer class to make it easier for other osl::Mutex->std::mutex conversions. Change-Id: I5dbd84aa9b7fd9c5e6058d23b993b732f61fbd20 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120846 Tested-by: Jenkins Reviewed-by: Noel Grandin --- comphelper/source/misc/accessibleeventnotifier.cxx | 65 ++-- include/comphelper/interfacecontainer4.hxx | 328 +++++++++++++++++++++ 2 files changed, 355 insertions(+), 38 deletions(-) create mode 100644 include/comphelper/interfacecontainer4.hxx diff --git a/comphelper/source/misc/accessibleeventnotifier.cxx b/comphelper/source/misc/accessibleeventnotifier.cxx index d2694ab6120b..c1e26c35bdd9 100644 --- a/comphelper/source/misc/accessibleeventnotifier.cxx +++ b/comphelper/source/misc/accessibleeventnotifier.cxx @@ -19,7 +19,7 @@ #include #include -#include +#include #include #include @@ -35,16 +35,16 @@ namespace { typedef std::pair< AccessibleEventNotifier::TClientId, AccessibleEventObject > ClientEvent; -typedef std::map< AccessibleEventNotifier::TClientId, - ::comphelper::OInterfaceContainerHelper2* > ClientMap; +typedef ::comphelper::OInterfaceContainerHelper4 ListenerContainer; +typedef std::map< AccessibleEventNotifier::TClientId, ListenerContainer* > ClientMap; /// key is the end of the interval, value is the start of the interval typedef std::map IntervalMap; -::osl::Mutex& GetLocalMutex() +std::mutex& GetLocalMutex() { - static ::osl::Mutex MUTEX; + static std::mutex MUTEX; return MUTEX; } @@ -149,19 +149,13 @@ namespace comphelper { AccessibleEventNotifier::TClientId AccessibleEventNotifier::registerClient() { - ::osl::MutexGuard aGuard( GetLocalMutex() ); + std::scoped_lock aGuard( GetLocalMutex() ); // generate a new client id TClientId nNewClientId = generateId( ); // the event listeners for the new client - ::comphelper::OInterfaceContainerHelper2 *const pNewListeners = - new ::comphelper::OInterfaceContainerHelper2( GetLocalMutex() ); - // note that we're using our own mutex here, so the listener containers for all - // our clients share this same mutex. - // this is a reminiscence to the days where the notifier was asynchronous. Today this is - // completely nonsense, and potentially slowing down the Office me thinks... - + ListenerContainer * pNewListeners = new ListenerContainer(); // add the client gaClients.emplace( nNewClientId, pNewListeners ); @@ -171,7 +165,7 @@ AccessibleEventNotifier::TClientId AccessibleEventNotifier::registerClient() void AccessibleEventNotifier::revokeClient( const TClientId _nClient ) { - ::osl::MutexGuard aGuard( GetLocalMutex() ); + std::scoped_lock aGuard( GetLocalMutex() ); ClientMap::iterator aClientPos; if ( !implLookupClient( _nClient, aClientPos ) ) @@ -187,40 +181,35 @@ void AccessibleEventNotifier::revokeClient( const TClientId _nClient ) void AccessibleEventNotifier::revokeClientNotifyDisposing( const TClientId _nClient, const Reference< XInterface >& _rxEventSource ) { - std::unique_ptr<::comphelper::OInterfaceContainerHelper2> pListeners; - - { - // rhbz#1001768 drop the mutex before calling disposeAndClear - ::osl::MutexGuard aGuard( GetLocalMutex() ); + std::unique_lock aGuard( GetLocalMutex() ); - ClientMap::iterator aClientPos; - if (!implLookupClient(_nClient, aClientPos)) - // already asserted in implLookupClient - return; + ClientMap::iterator aClientPos; + if (!implLookupClient(_nClient, aClientPos)) + // already asserted in implLookupClient + return; - // notify the listeners - pListeners.reset(aClientPos->second); + // notify the listeners + std::unique_ptr pListeners(aClientPos->second); - // we do not need the entry in the clients map anymore - // (do this before actually notifying, because some client - // implementations have re-entrance problems and call into - // revokeClient while we are notifying from here) - gaClients.erase(aClientPos); - releaseId(_nClient); - } + // we do not need the entry in the clients map anymore + // (do this before actually notifying, because some client + // implementations have re-entrance problems and call into + // revokeClient while we are notifying from here) + gaClients.erase(aClientPos); + releaseId(_nClient); // notify the "disposing" event for this client EventObject aDisposalEvent; aDisposalEvent.Source = _rxEventSource; // now really do the notification - pListeners->disposeAndClear( aDisposalEvent ); + pListeners->disposeAndClear( aGuard, aDisposalEvent ); } sal_Int32 AccessibleEventNotifier::addEventListener( const TClientId _nClient, const Reference< XAccessibleEventListener >& _rxListener ) { - ::osl::MutexGuard aGuard( GetLocalMutex() ); + std::scoped_lock aGuard( GetLocalMutex() ); ClientMap::iterator aClientPos; if ( !implLookupClient( _nClient, aClientPos ) ) @@ -236,7 +225,7 @@ sal_Int32 AccessibleEventNotifier::addEventListener( sal_Int32 AccessibleEventNotifier::removeEventListener( const TClientId _nClient, const Reference< XAccessibleEventListener >& _rxListener ) { - ::osl::MutexGuard aGuard( GetLocalMutex() ); + std::scoped_lock aGuard( GetLocalMutex() ); ClientMap::iterator aClientPos; if ( !implLookupClient( _nClient, aClientPos ) ) @@ -251,10 +240,10 @@ sal_Int32 AccessibleEventNotifier::removeEventListener( void AccessibleEventNotifier::addEvent( const TClientId _nClient, const AccessibleEventObject& _rEvent ) { - std::vector< Reference< XInterface > > aListeners; + std::vector< Reference< XAccessibleEventListener > > aListeners; { - ::osl::MutexGuard aGuard( GetLocalMutex() ); + std::scoped_lock aGuard( GetLocalMutex() ); ClientMap::iterator aClientPos; if ( !implLookupClient( _nClient, aClientPos ) ) @@ -270,7 +259,7 @@ void AccessibleEventNotifier::addEvent( const TClientId _nClient, const Accessib { try { - static_cast< XAccessibleEventListener* >( rListener.get() )->notifyEvent( _rEvent ); + rListener->notifyEvent( _rEvent ); } catch( const Exception& ) { diff --git a/include/comphelper/interfacecontainer4.hxx b/include/comphelper/interfacecontainer4.hxx new file mode 100644 index 000000000000..0260ac4f5c0b --- /dev/null +++ b/include/comphelper/interfacecontainer4.hxx @@ -0,0 +1,328 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + * + * This file incorporates work covered by the following license notice: + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed + * with this work for additional information regarding copyright + * ownership. The ASF licenses this file to you under the Apache + * License, Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of + * the License at http://www.apache.org/licenses/LICENSE-2.0 . + */ +#ifndef INCLUDED_COMPHELPER_INTERFACECONTAINER3_H +#define INCLUDED_COMPHELPER_INTERFACECONTAINER3_H + +#include + +#include +#include +#include +#include +#include + +namespace com::sun::star::uno +{ +class XInterface; +} + +/** */ //for docpp +namespace comphelper +{ +template class OInterfaceContainerHelper4; +/** + This is the iterator of an OInterfaceContainerHelper4. Typically + one constructs an instance on the stack for one firing session. + It is not allowed to assign or copy an instance of this class. + + @tparam ListenerT UNO event listener type + @see OInterfaceContainerHelper4 + */ +template class OInterfaceIteratorHelper4 +{ +public: + /** + Create an iterator over the elements of the container. The iterator + copies the elements of the container. A change to the container + during the lifetime of an iterator is allowed and does not + affect the iterator-instance. The iterator and the container take cares + themself for concurrent access, no additional guarding is necessary. + + Remark: The copy is on demand. The iterator copy the elements only if the container + change the contents... + + @param rCont the container of the elements. + */ + OInterfaceIteratorHelper4(OInterfaceContainerHelper4& rCont_) + : rCont(rCont_) + , maData(rCont.maData) + , nRemain(maData->size()) + { + } + + /** Return true, if there are more elements in the iterator. */ + bool hasMoreElements() const { return nRemain != 0; } + /** Return the next element of the iterator. Calling this method if + hasMoreElements() has returned false, is an error. + */ + css::uno::Reference const& next(); + + /** Removes the current element (the last one returned by next()) + from the underlying container. Calling this method before + next() has been called or calling it twice with no next() + in between is an error. + */ + void remove(); + +private: + OInterfaceContainerHelper4& rCont; + o3tl::cow_wrapper>> maData; + sal_Int32 nRemain; + + OInterfaceIteratorHelper4(const OInterfaceIteratorHelper4&) = delete; + OInterfaceIteratorHelper4& operator=(const OInterfaceIteratorHelper4&) = delete; +}; + +template +const css::uno::Reference& OInterfaceIteratorHelper4::next() +{ + nRemain--; + return (*maData)[nRemain]; +} + +template void OInterfaceIteratorHelper4::remove() +{ + rCont.removeInterface((*maData)[nRemain]); +} + +/** + A container of interfaces. To access the elements use an iterator. + This implementation is thread-safe. + + This is a copy of the code at include/comphelper/interfacecontainer3.hxx, + except that it (a) uses std::mutex instead of osl::Mutex and (b) does not + store a reference to the mutex, but relies on the calling class to take + a lock around using it. + + @tparam ListenerT UNO event listener type + @see OInterfaceIteratorHelper + */ +template class OInterfaceContainerHelper4 +{ +public: + OInterfaceContainerHelper4() {} + /** + Return the number of Elements in the container. Only useful if you have acquired + the mutex. + */ + sal_Int32 getLength() const; + + /** + Return all interfaces added to this container. + **/ + std::vector> getElements() const; + + /** Inserts an element into the container. The position is not specified, thus it is not + specified in which order events are fired. + + @attention + If you add the same interface more than once, then it will be added to the elements list + more than once and thus if you want to remove that interface from the list, you have to call + removeInterface() the same number of times. + In the latter case, you will also get events fired more than once (if the interface is a + listener interface). + + @param rxIFace + interface to be added; it is allowed to insert + the same interface more than once + @return + the new count of elements in the container + */ + sal_Int32 addInterface(const css::uno::Reference& rxIFace); + /** Removes an element from the container. It uses interface equality to remove the interface. + + @param rxIFace + interface to be removed + @return + the new count of elements in the container + */ + sal_Int32 removeInterface(const css::uno::Reference& rxIFace); + /** + Call disposing on all object in the container that + support XEventListener. Then clear the container. + The guard is unlock()'ed before calling the listeners. + */ + void disposeAndClear(::std::unique_lock<::std::mutex>& rGuard, + const css::lang::EventObject& rEvt); + /** + Clears the container without calling disposing(). + */ + void clear(); + + /** Executes a functor for each contained listener of specified type, e.g. + forEach(.... + + If a css::lang::DisposedException occurs which relates to + the called listener, then that listener is removed from the container. + + @tparam FuncT unary functor type, let your compiler deduce this for you + @param func unary functor object expecting an argument of type + css::uno::Reference + */ + template inline void forEach(FuncT const& func); + + /** Calls a UNO listener method for each contained listener. + + The listener method must take a single argument of type EventT, + and return void. + + If a css::lang::DisposedException occurs which relates to + the called listener, then that listener is removed from the container. + + @tparam EventT event type, let your compiler deduce this for you + @param NotificationMethod + Pointer to a method of a ListenerT interface. + @param Event + Event to notify to all contained listeners + + Example: +@code + awt::PaintEvent aEvent( static_cast< cppu::OWeakObject* >( this ), ... ); + listeners.notifyEach( &XPaintListener::windowPaint, aEvent ); +@endcode + */ + template + inline void notifyEach(void (SAL_CALL ListenerT::*NotificationMethod)(const EventT&), + const EventT& Event); + +private: + friend class OInterfaceIteratorHelper4; + o3tl::cow_wrapper>> maData; + OInterfaceContainerHelper4(const OInterfaceContainerHelper4&) = delete; + OInterfaceContainerHelper4& operator=(const OInterfaceContainerHelper4&) = delete; + +private: + template class NotifySingleListener + { + private: + typedef void (SAL_CALL ListenerT::*NotificationMethod)(const EventT&); + NotificationMethod const m_pMethod; + const EventT& m_rEvent; + + public: + NotifySingleListener(NotificationMethod method, const EventT& event) + : m_pMethod(method) + , m_rEvent(event) + { + } + + void operator()(const css::uno::Reference& listener) const + { + (listener.get()->*m_pMethod)(m_rEvent); + } + }; +}; + +template +template +inline void OInterfaceContainerHelper4::forEach(FuncT const& func) +{ + OInterfaceIteratorHelper4 iter(*this); + while (iter.hasMoreElements()) + { + auto xListener = iter.next(); + try + { + func(xListener); + } + catch (css::lang::DisposedException const& exc) + { + if (exc.Context == xListener) + iter.remove(); + } + } +} + +template +template +inline void OInterfaceContainerHelper4::notifyEach( + void (SAL_CALL ListenerT::*NotificationMethod)(const EventT&), const EventT& Event) +{ + forEach>(NotifySingleListener(NotificationMethod, Event)); +} + +template sal_Int32 OInterfaceContainerHelper4::getLength() const +{ + return maData->size(); +} + +template +std::vector> +OInterfaceContainerHelper4::getElements() const +{ + return *maData; +} + +template +sal_Int32 +OInterfaceContainerHelper4::addInterface(const css::uno::Reference& rListener) +{ + assert(rListener.is()); + maData->push_back(rListener); + return maData->size(); +} + +template +sal_Int32 OInterfaceContainerHelper4::removeInterface( + const css::uno::Reference& rListener) +{ + assert(rListener.is()); + + // It is not valid to compare the pointer directly, but it's faster. + auto it = std::find_if(maData->begin(), maData->end(), + [&rListener](const css::uno::Reference& rItem) { + return rItem.get() == rListener.get(); + }); + + // interface not found, use the correct compare method + if (it == maData->end()) + it = std::find(maData->begin(), maData->end(), rListener); + + if (it != maData->end()) + maData->erase(it); + + return maData->size(); +} + +template +void OInterfaceContainerHelper4::disposeAndClear(std::unique_lock& rGuard, + const css::lang::EventObject& rEvt) +{ + OInterfaceIteratorHelper4 aIt(*this); + maData->clear(); + rGuard.unlock(); + while (aIt.hasMoreElements()) + { + try + { + aIt.next()->disposing(rEvt); + } + catch (css::uno::RuntimeException&) + { + // be robust, if e.g. a remote bridge has disposed already. + // there is no way to delegate the error to the caller :o(. + } + } +} + +template void OInterfaceContainerHelper4::clear() { maData->clear(); } +} +#endif + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ -- cgit