comphelper/source/misc/asyncnotification.cxx | 96 ++++---------- compilerplugins/clang/compat.hxx | 8 + dbaccess/source/core/dataaccess/documenteventnotifier.cxx | 10 - desktop/scripts/soffice.sh | 2 include/comphelper/asyncnotification.hxx | 4 solenv/gbuild/CppunitTest.mk | 2 6 files changed, 45 insertions(+), 77 deletions(-)
New commits: commit 9b7f17dd3087d6926ee51e4d66f0ce3a3ab90867 Author: Stephan Bergmann <sberg...@redhat.com> Date: Fri Mar 14 12:09:21 2014 +0100 Fix races in AsyncEventNotifier::execute * m_aDeadProcessors was useless; for one, removeEventsForProcessor could never have run (and set m_aDeadProcessors) between execute's reading from aEvents and checking m_aDeadProcessors (because of the use of aMutex in both functions), and if that were addressed, there would always be a race that execute would run a processor that has just been removed. Clients have to be aware that a call to removeEventsForProcessor is just an optimization hint, but does not guarantee that the given processor is not called from the execute thread after removeEventsForProcessor returns. * The sequence aGuard.clear(); aPendingActions.reset(); aPanedingActions.wait(); could cause calls to aPendingActions.set() to get lost, both for signalling new content in the queue and for signalling termination. Change-Id: I43293e3d5090c2d46db8bc8ed6fb9c71e049d55c diff --git a/comphelper/source/misc/asyncnotification.cxx b/comphelper/source/misc/asyncnotification.cxx index d419580..a8c018c 100644 --- a/comphelper/source/misc/asyncnotification.cxx +++ b/comphelper/source/misc/asyncnotification.cxx @@ -23,8 +23,8 @@ #include <osl/conditn.hxx> #include <comphelper/guarding.hxx> +#include <cassert> #include <deque> -#include <set> #include <functional> #include <algorithm> @@ -72,23 +72,14 @@ namespace comphelper AnyEventRef aEvent; ::rtl::Reference< IEventProcessor > xProcessor; - ProcessableEvent( const AnyEventRef& _rEvent, const ::rtl::Reference< IEventProcessor >& _xProcessor ) - :aEvent( _rEvent ) - ,xProcessor( _xProcessor ) + ProcessableEvent() { } - ProcessableEvent( const ProcessableEvent& _rRHS ) - :aEvent( _rRHS.aEvent ) - ,xProcessor( _rRHS.xProcessor ) - { - } - - ProcessableEvent& operator=( const ProcessableEvent& _rRHS ) + ProcessableEvent( const AnyEventRef& _rEvent, const ::rtl::Reference< IEventProcessor >& _xProcessor ) + :aEvent( _rEvent ) + ,xProcessor( _xProcessor ) { - aEvent = _rRHS.aEvent; - xProcessor = _rRHS.xProcessor; - return *this; } }; @@ -113,20 +104,14 @@ namespace comphelper struct EventNotifierImpl { ::osl::Mutex aMutex; - oslInterlockedCount m_refCount; ::osl::Condition aPendingActions; EventQueue aEvents; - ::std::set< ::rtl::Reference< IEventProcessor > > - m_aDeadProcessors; + bool bTerminate; EventNotifierImpl() - :m_refCount( 0 ) + :bTerminate( false ) { } - - private: - EventNotifierImpl( const EventNotifierImpl& ); // never implemented - EventNotifierImpl& operator=( const EventNotifierImpl& ); // never implemented }; @@ -150,10 +135,6 @@ namespace comphelper // remove all events for this processor ::std::remove_if( m_pImpl->aEvents.begin(), m_pImpl->aEvents.end(), EqualProcessor( _xProcessor ) ); - - // and just in case that an event for exactly this processor has just been - // popped from the queue, but not yet processed: remember it: - m_pImpl->m_aDeadProcessors.insert( _xProcessor ); } @@ -162,7 +143,7 @@ namespace comphelper ::osl::MutexGuard aGuard( m_pImpl->aMutex ); // remember the termination request - Thread::terminate(); + m_pImpl->bTerminate = true; // awake the thread m_pImpl->aPendingActions.set(); @@ -184,57 +165,36 @@ namespace comphelper void AsyncEventNotifier::execute() { - do + for (;;) { - AnyEventRef aNextEvent; - ::rtl::Reference< IEventProcessor > xNextProcessor; - - ::osl::ClearableMutexGuard aGuard( m_pImpl->aMutex ); - while ( m_pImpl->aEvents.size() > 0 ) + m_pImpl->aPendingActions.wait(); + ProcessableEvent aEvent; { - ProcessableEvent aEvent( m_pImpl->aEvents.front() ); - aNextEvent = aEvent.aEvent; - xNextProcessor = aEvent.xProcessor; - m_pImpl->aEvents.pop_front(); - - OSL_TRACE( "AsyncEventNotifier(%p): popping %p", this, aNextEvent.get() ); - - if ( !aNextEvent.get() ) - continue; - - // process the event, but only if it's processor did not die inbetween - ::std::set< ::rtl::Reference< IEventProcessor > >::iterator deadPos = m_pImpl->m_aDeadProcessors.find( xNextProcessor ); - if ( deadPos != m_pImpl->m_aDeadProcessors.end() ) + osl::MutexGuard aGuard(m_pImpl->aMutex); + if (m_pImpl->bTerminate) { - m_pImpl->m_aDeadProcessors.erase( xNextProcessor ); - xNextProcessor.clear(); - OSL_TRACE( "AsyncEventNotifier(%p): removing %p", this, aNextEvent.get() ); + break; } - - // if there was a termination request (->terminate), respect it - if ( !schedule() ) - return; - + if (!m_pImpl->aEvents.empty()) + { + aEvent = m_pImpl->aEvents.front(); + m_pImpl->aEvents.pop_front(); + OSL_TRACE( + "AsyncEventNotifier(%p): popping %p", this, + aEvent.aEvent.get()); + } + if (m_pImpl->aEvents.empty()) { - ::comphelper::MutexRelease aReleaseOnce( m_pImpl->aMutex ); - if ( xNextProcessor.get() ) - xNextProcessor->processEvent( *aNextEvent.get() ); + m_pImpl->aPendingActions.reset(); } } - - // if there was a termination request (->terminate), respect it - if ( !schedule() ) - return; - - // wait for new events to process - aGuard.clear(); - m_pImpl->aPendingActions.reset(); - m_pImpl->aPendingActions.wait(); + if (aEvent.aEvent.is()) { + assert(aEvent.xProcessor.is()); + aEvent.xProcessor->processEvent(*aEvent.aEvent); + } } - while ( true ); } - } // namespace comphelper commit ddfe9eec96ffe322b4952c25e2c3209da3e44a39 Author: Stephan Bergmann <sberg...@redhat.com> Date: Fri Mar 14 12:01:42 2014 +0100 Don't Valgrind into java children in unit tests ...similarly to desktop/scripts/soffice.sh preventing that for LO itself. One problem is that, even if it does not find any errors, Valgrind writes onto a traced child's stderr, and when jvmfwk searches for a JRE to use, it takes output to stderr into account, so would start to behave differently when run under Valgrind. --trace-children-skip appears to be new since Valgrind 3.6, and older versions would probably fail early when they see this unknown-to-them option. If this turns out to be a problem in practice (current version is 3.9), we probably need to make this conditional on a configure.ac check. Change-Id: Ia6a72bdcd666d68ed0539170f3fc476292e82b96 diff --git a/solenv/gbuild/CppunitTest.mk b/solenv/gbuild/CppunitTest.mk index ba3ddac..a227540 100644 --- a/solenv/gbuild/CppunitTest.mk +++ b/solenv/gbuild/CppunitTest.mk @@ -27,7 +27,7 @@ gb_CppunitTest__interactive := $(true) endif ifneq ($(strip $(VALGRIND)),) -gb_CppunitTest_VALGRINDTOOL := valgrind --tool=$(VALGRIND) --num-callers=50 --error-exitcode=1 --trace-children=yes --leak-check=no +gb_CppunitTest_VALGRINDTOOL := valgrind --tool=$(VALGRIND) --num-callers=50 --error-exitcode=1 --trace-children=yes --trace-children-skip='*/java,*/gij' --leak-check=no ifeq ($(strip $(VALGRIND)),memcheck) G_SLICE := always-malloc GLIBCXX_FORCE_NEW := 1 commit 3ceaeff1cfbd606c4839832d8558617c0513be1d Author: Stephan Bergmann <sberg...@redhat.com> Date: Fri Mar 14 11:59:58 2014 +0100 Skip gcj's gij executable, like other JREs' java executables Change-Id: Ida84a271a066e89ceb7e5dd2fd23a744f5529917 diff --git a/desktop/scripts/soffice.sh b/desktop/scripts/soffice.sh index d08f52a..6d5dc38 100755 --- a/desktop/scripts/soffice.sh +++ b/desktop/scripts/soffice.sh @@ -101,7 +101,7 @@ for arg in $@ $VALGRINDOPT ; do valgrind_ver_min=`echo $valgrind_ver | awk -F. '{ print \$2 }'` valgrind_skip= if [ "$valgrind_ver_maj" -gt 3 -o \( "$valgrind_ver_maj" -eq 3 -a "$valgrind_ver_min" -ge 6 \) ] ; then - valgrind_skip='--trace-children-skip=*/java' + valgrind_skip='--trace-children-skip=*/java,*/gij' fi # finally set the valgrind check VALGRINDCHECK="valgrind --tool=$VALGRIND --trace-children=yes $valgrind_skip --num-callers=50 --error-limit=no" commit f6ff4c955a2c7dad8d586c1ae351856f596d7c76 Author: Stephan Bergmann <sberg...@redhat.com> Date: Fri Mar 14 11:59:07 2014 +0100 More compat stuff (currently only used by a not-yet committed plugin, though) Change-Id: Id62ea41031ad8ba4495ef46877ad7a10bc58fb05 diff --git a/compilerplugins/clang/compat.hxx b/compilerplugins/clang/compat.hxx index fa89eba..f100f25 100644 --- a/compilerplugins/clang/compat.hxx +++ b/compilerplugins/clang/compat.hxx @@ -51,6 +51,14 @@ inline clang::QualType getReturnType(clang::FunctionDecl const & decl) { #endif } +inline clang::QualType getReturnType(clang::FunctionProtoType const & type) { +#if (__clang_major__ == 3 && __clang_minor__ >= 5) || __clang_major__ > 3 + return type.getReturnType(); +#else + return type.getResultType(); +#endif +} + inline unsigned getNumParams(clang::FunctionProtoType const & type) { #if (__clang_major__ == 3 && __clang_minor__ >= 5) || __clang_major__ > 3 return type.getNumParams(); commit 0085bd2866424473de288c4bc6fce31323be5284 Author: Stephan Bergmann <sberg...@redhat.com> Date: Fri Mar 14 11:53:40 2014 +0100 Model IEventProcessor acquire/release exactly after XInterface ...so classes deriving from both can easily share a single implementation for these functions. Change-Id: I6882dddc8b3ea3b0192d85102a0305494d964dc1 diff --git a/dbaccess/source/core/dataaccess/documenteventnotifier.cxx b/dbaccess/source/core/dataaccess/documenteventnotifier.cxx index 0f97771..a556760 100644 --- a/dbaccess/source/core/dataaccess/documenteventnotifier.cxx +++ b/dbaccess/source/core/dataaccess/documenteventnotifier.cxx @@ -74,9 +74,9 @@ namespace dbaccess { } - // IReference - virtual void SAL_CALL acquire(); - virtual void SAL_CALL release(); + // IEventProcessor + virtual void SAL_CALL acquire() throw (); + virtual void SAL_CALL release() throw (); void addLegacyEventListener( const Reference< document::XEventListener >& _Listener ) { @@ -129,12 +129,12 @@ namespace dbaccess void impl_notifyEventAsync_nothrow( const DocumentEvent& _rEvent ); }; - void SAL_CALL DocumentEventNotifier_Impl::acquire() + void SAL_CALL DocumentEventNotifier_Impl::acquire() throw () { osl_atomic_increment( &m_refCount ); } - void SAL_CALL DocumentEventNotifier_Impl::release() + void SAL_CALL DocumentEventNotifier_Impl::release() throw () { if ( 0 == osl_atomic_decrement( &m_refCount ) ) delete this; diff --git a/include/comphelper/asyncnotification.hxx b/include/comphelper/asyncnotification.hxx index f33d823..40aa21a 100644 --- a/include/comphelper/asyncnotification.hxx +++ b/include/comphelper/asyncnotification.hxx @@ -76,8 +76,8 @@ namespace comphelper */ virtual void processEvent( const AnyEvent& _rEvent ) = 0; - virtual void SAL_CALL acquire() = 0; - virtual void SAL_CALL release() = 0; + virtual void SAL_CALL acquire() throw () = 0; + virtual void SAL_CALL release() throw () = 0; protected: ~IEventProcessor() {} _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits