From 1de218111cf1b814d939e810264895c004fe3bf9 Mon Sep 17 00:00:00 2001 From: "Frank Schoenheit [fs]" Date: Thu, 16 Sep 2010 14:25:54 +0200 Subject: dba34a: #i111148# do not call setPropertyValue from within setFastPropertyValue_NoBroadcast, this is prone to deadlocks. Instead, use the newly introduced setDependentFastPropertyValue, which postpones the notifications to a more appropriate point in time --- cppuhelper/inc/cppuhelper/propshlp.hxx | 35 ++++++++++- cppuhelper/source/cc5_solaris_sparc.map | 6 ++ cppuhelper/source/gcc3.map | 6 ++ cppuhelper/source/msvc_win32_intel.map | 6 ++ cppuhelper/source/propshlp.cxx | 105 +++++++++++++++++++++++++++++--- 5 files changed, 149 insertions(+), 9 deletions(-) (limited to 'cppuhelper') diff --git a/cppuhelper/inc/cppuhelper/propshlp.hxx b/cppuhelper/inc/cppuhelper/propshlp.hxx index 61c8a2269..ea9a13877 100644 --- a/cppuhelper/inc/cppuhelper/propshlp.hxx +++ b/cppuhelper/inc/cppuhelper/propshlp.hxx @@ -595,6 +595,29 @@ protected: ::com::sun::star::uno::Any& rValue, sal_Int32 nHandle ) const = 0; + /** sets an dependent property's value + +

Sometimes setting a given property needs to implicitly modify another property's value. Calling |setPropertyValue| + from within |setFastPropertyValue_NoBroadcast| is not an option here, as it would notify the property listeners + while our mutex is still locked. Setting the dependent property's value directly (e.g. by calling |setFastPropertyValue_NoBroadcast| + recursively) is not an option, too, since it would miss firing the property change event.

+ +

So, in such cases, you use |setDependentFastPropertyValue| from within |setFastPropertyValue_NoBroadcast|. + It will convert and actually set the property value (invoking |convertFastPropertyValue| and |setFastPropertyValue_NoBroadcast| + for the given handle and value), and add the property change event to the list of events to be notified + when the bottom-most |setFastPropertyValue_NoBroadcast| on the stack returns.

+ +

Note: The method will not invoke veto listeners for the property.

+ +

Note: It's the caller's responsibility to ensure that our mutex is locked. This is + canonically given when the method is invoked from within |setFastPropertyValue_NoBroadcast|, in other + contexts, you might need to take own measures.

+ */ + void setDependentFastPropertyValue( + sal_Int32 i_handle, + const ::com::sun::star::uno::Any& i_value + ); + /** The common data of a broadcaster. Use the mutex, disposing state and the listener container. */ OBroadcastHelper &rBHelper; /** @@ -610,12 +633,22 @@ protected: /** reserved for future use. finally, the future has arrived... */ - const std::auto_ptr m_pReserved; + const std::auto_ptr m_pReserved; private: OPropertySetHelper( const OPropertySetHelper & ) SAL_THROW( () ); OPropertySetHelper & operator = ( const OPropertySetHelper & ) SAL_THROW( () ); + /** notifies the given changes in property's values, plus all property changes collected during recent + |setDependentFastPropertyValue| calls. + */ + void impl_fireAll( + sal_Int32* i_handles, + const ::com::sun::star::uno::Any * i_newValues, + const ::com::sun::star::uno::Any * i_oldValues, + sal_Int32 i_count + ); + public: // Suppress warning about virtual functions but non-virtual destructor: #if defined __GNUC__ diff --git a/cppuhelper/source/cc5_solaris_sparc.map b/cppuhelper/source/cc5_solaris_sparc.map index 090f8a1cc..2202d6fa8 100755 --- a/cppuhelper/source/cc5_solaris_sparc.map +++ b/cppuhelper/source/cc5_solaris_sparc.map @@ -387,3 +387,9 @@ UDK_3.7 { # OOo 3.3 __1cDcomDsunEstarDunoTWeakReferenceHelperFclear6M_v_; __1cEcppubHcreateOneInstanceComponentFactory6FpFrknDcomDsunEstarDunoJReference4n0ERXComponentContext____n0EJReference4n0EKXInterface___rknDrtlIOUString_rkn0EISequence4n0K___pnQ_rtl_ModuleCount__n0EJReference4n0DElangXXSingleComponentFactory____; } UDK_3.6; + +UDK_3.8 { # OOo 3.4 + global: + __1cEcppuSOPropertySetHelperbDsetDependentFastPropertyValue6MklrknDcomDsunEstarDunoDAny__v_; +} UDK_3.7; + diff --git a/cppuhelper/source/gcc3.map b/cppuhelper/source/gcc3.map index 88e58df65..f842854e2 100644 --- a/cppuhelper/source/gcc3.map +++ b/cppuhelper/source/gcc3.map @@ -382,3 +382,9 @@ UDK_3.6 { # OOo 3.3 _ZN4cppu33createOneInstanceComponentFactoryEPFN3com3sun4star3uno9ReferenceINS3_10XInterfaceEEERKNS4_INS3_17XComponentContextEEEERKN3rtl8OUStringERKNS3_8SequenceISE_EEP16_rtl_ModuleCount; } UDK_3.5; + +UDK_3.7 { # OOo 3.4 + global: + _ZN4cppu18OPropertySetHelper29setDependentFastPropertyValueElRKN3com3sun4star3uno3AnyE; +} UDK_3.6; + diff --git a/cppuhelper/source/msvc_win32_intel.map b/cppuhelper/source/msvc_win32_intel.map index 6d5a491ab..7069276e2 100644 --- a/cppuhelper/source/msvc_win32_intel.map +++ b/cppuhelper/source/msvc_win32_intel.map @@ -278,3 +278,9 @@ UDK_3.6 { # OOo 3.3 ?clear@WeakReferenceHelper@uno@star@sun@com@@QAAXXZ; ?createOneInstanceComponentFactory@cppu@@YA?AV?$Reference@VXSingleComponentFactory@lang@star@sun@com@@@uno@star@sun@com@@P6A?AV?$Reference@VXInterface@uno@star@sun@com@@@3456@ABV?$Reference@VXComponentContext@uno@star@sun@com@@@3456@@ZABVOUString@rtl@@ABV?$Sequence@VOUString@rtl@@@3456@PAU_rtl_ModuleCount@@@Z; } UDK_3.5; + +UDK_3.7 { # OOo 3.4 + global: + ?setDependentFastPropertyValue@OPropertySetHelper@cppu@@IAEXJABVAny@uno@star@sun@com@@@Z; +} UDK_3.6; + diff --git a/cppuhelper/source/propshlp.cxx b/cppuhelper/source/propshlp.cxx index eb71723e1..fc45ccf89 100644 --- a/cppuhelper/source/propshlp.cxx +++ b/cppuhelper/source/propshlp.cxx @@ -32,6 +32,7 @@ #include "cppuhelper/implbase1.hxx" #include "cppuhelper/weak.hxx" #include "cppuhelper/propshlp.hxx" +#include "cppuhelper/exc_hlp.hxx" #include "com/sun/star/beans/PropertyAttribute.hpp" #include "com/sun/star/lang/DisposedException.hpp" @@ -144,15 +145,20 @@ sal_Bool OPropertySetHelperInfo_Impl::hasPropertyByName( const OUString & Proper class OPropertySetHelper::Impl { public: - Impl ( bool i_bIgnoreRuntimeExceptionsWhileFiring, - IEventNotificationHook *i_pFireEvents) - : m_bIgnoreRuntimeExceptionsWhileFiring( - i_bIgnoreRuntimeExceptionsWhileFiring ), - m_pFireEvents( i_pFireEvents ) - { } + Impl( bool i_bIgnoreRuntimeExceptionsWhileFiring, + IEventNotificationHook *i_pFireEvents + ) + :m_bIgnoreRuntimeExceptionsWhileFiring( i_bIgnoreRuntimeExceptionsWhileFiring ) + ,m_pFireEvents( i_pFireEvents ) + { + } bool m_bIgnoreRuntimeExceptionsWhileFiring; class IEventNotificationHook * const m_pFireEvents; + + ::std::vector< sal_Int32 > m_handles; + ::std::vector< Any > m_newValues; + ::std::vector< Any > m_oldValues; }; @@ -432,6 +438,57 @@ void OPropertySetHelper::removeVetoableChangeListener( } } +void OPropertySetHelper::setDependentFastPropertyValue( sal_Int32 i_handle, const ::com::sun::star::uno::Any& i_value ) +{ + //OSL_PRECOND( rBHelper.rMutex.isAcquired(), "OPropertySetHelper::setDependentFastPropertyValue: to be called with a locked mutex only!" ); + // there is no such thing as Mutex.isAcquired, sadly ... + + sal_Int16 nAttributes(0); + IPropertyArrayHelper& rInfo = getInfoHelper(); + if ( !rInfo.fillPropertyMembersByHandle( NULL, &nAttributes, i_handle ) ) + // unknown property + throw UnknownPropertyException(); + + // no need to check for READONLY-ness of the property. The method is intended to be called internally, which + // implies it might be invoked for properties which are read-only to the instance's clients, but well allowed + // to change their value. + + Any aConverted, aOld; + sal_Bool bChanged = convertFastPropertyValue( aConverted, aOld, i_handle, i_value ); + if ( !bChanged ) + return; + + // don't fire vetoable events. This method is called with our mutex locked, so calling into listeners would not be + // a good idea. The caler is responsible for not invoking this for constrained properties. + OSL_ENSURE( ( nAttributes & PropertyAttribute::CONSTRAINED ) == 0, + "OPropertySetHelper::setDependentFastPropertyValue: not to be used for constrained properties!" ); + (void)nAttributes; + + // actually set the new value + try + { + setFastPropertyValue_NoBroadcast( i_handle, aConverted ); + } + catch (const UnknownPropertyException& ) { throw; /* allowed to leave */ } + catch (const PropertyVetoException& ) { throw; /* allowed to leave */ } + catch (const IllegalArgumentException& ) { throw; /* allowed to leave */ } + catch (const WrappedTargetException& ) { throw; /* allowed to leave */ } + catch (const RuntimeException& ) { throw; /* allowed to leave */ } + catch (const Exception& ) + { + // not allowed to leave this meathod + WrappedTargetException aWrapped; + aWrapped.TargetException <<= ::cppu::getCaughtException(); + aWrapped.Context = static_cast< XPropertySet* >( this ); + throw aWrapped; + } + + // remember the handle/values, for the events to be fired later + m_pReserved->m_handles.push_back( i_handle ); + m_pReserved->m_newValues.push_back( aConverted ); // TODO: setFastPropertyValue notifies the unconverted value here ...? + m_pReserved->m_oldValues.push_back( aOld ); +} + // XFastPropertySet void OPropertySetHelper::setFastPropertyValue( sal_Int32 nHandle, const Any& rValue ) throw(::com::sun::star::beans::UnknownPropertyException, @@ -498,7 +555,7 @@ void OPropertySetHelper::setFastPropertyValue( sal_Int32 nHandle, const Any& rVa // release guard to fire events } // file a change event, if the value changed - fire( &nHandle, &rValue, &aOldVal, 1, sal_False ); + impl_fireAll( &nHandle, &rValue, &aOldVal, 1 ); } } @@ -520,6 +577,38 @@ Any OPropertySetHelper::getFastPropertyValue( sal_Int32 nHandle ) return aRet; } +//-------------------------------------------------------------------------- +void OPropertySetHelper::impl_fireAll( sal_Int32* i_handles, const Any* i_newValues, const Any* i_oldValues, sal_Int32 i_count ) +{ + ClearableMutexGuard aGuard( rBHelper.rMutex ); + if ( m_pReserved->m_handles.empty() ) + { + aGuard.clear(); + fire( i_handles, i_newValues, i_oldValues, i_count, sal_False ); + return; + } + + const size_t additionalEvents = m_pReserved->m_handles.size(); + OSL_ENSURE( additionalEvents == m_pReserved->m_newValues.size() + && additionalEvents == m_pReserved->m_oldValues.size(), + "OPropertySetHelper::impl_fireAll: inconsistency!" ); + + ::std::vector< sal_Int32 > allHandles( additionalEvents + i_count ); + ::std::copy( m_pReserved->m_handles.begin(), m_pReserved->m_handles.end(), allHandles.begin() ); + ::std::copy( i_handles, i_handles + i_count, allHandles.begin() + additionalEvents ); + + ::std::vector< Any > allNewValues( additionalEvents + i_count ); + ::std::copy( m_pReserved->m_newValues.begin(), m_pReserved->m_newValues.end(), allNewValues.begin() ); + ::std::copy( i_newValues, i_newValues + i_count, allNewValues.begin() + additionalEvents ); + + ::std::vector< Any > allOldValues( additionalEvents + i_count ); + ::std::copy( m_pReserved->m_oldValues.begin(), m_pReserved->m_oldValues.end(), allOldValues.begin() ); + ::std::copy( i_oldValues, i_oldValues + i_count, allOldValues.begin() + additionalEvents ); + + aGuard.clear(); + fire( &allHandles[0], &allNewValues[0], &allOldValues[0], additionalEvents + i_count, sal_False ); +} + //-------------------------------------------------------------------------- void OPropertySetHelper::fire ( @@ -803,7 +892,7 @@ void OPropertySetHelper::setFastPropertyValues( } // fire change events - fire( pHandles, pConvertedValues, pOldValues, n, sal_False ); + impl_fireAll( pHandles, pConvertedValues, pOldValues, n ); } catch( ... ) { -- cgit v1.2.3 From 2782beef38b449a8d54a3361709b3452ea754db1 Mon Sep 17 00:00:00 2001 From: "Frank Schoenheit [fs]" Date: Fri, 17 Sep 2010 07:47:49 +0200 Subject: dba34a: impl_fireAll: oops ... don't forget to reset handles/newValues/oldValues ... --- cppuhelper/source/propshlp.cxx | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'cppuhelper') diff --git a/cppuhelper/source/propshlp.cxx b/cppuhelper/source/propshlp.cxx index fc45ccf89..b0a41bb40 100644 --- a/cppuhelper/source/propshlp.cxx +++ b/cppuhelper/source/propshlp.cxx @@ -605,6 +605,10 @@ void OPropertySetHelper::impl_fireAll( sal_Int32* i_handles, const Any* i_newVal ::std::copy( m_pReserved->m_oldValues.begin(), m_pReserved->m_oldValues.end(), allOldValues.begin() ); ::std::copy( i_oldValues, i_oldValues + i_count, allOldValues.begin() + additionalEvents ); + m_pReserved->m_handles.clear(); + m_pReserved->m_newValues.clear(); + m_pReserved->m_oldValues.clear(); + aGuard.clear(); fire( &allHandles[0], &allNewValues[0], &allOldValues[0], additionalEvents + i_count, sal_False ); } -- cgit v1.2.3 From b9669e463f2e3a18f53a1f911dfc91c96fee265e Mon Sep 17 00:00:00 2001 From: "Frank Schoenheit [fs]" Date: Mon, 20 Sep 2010 11:00:07 +0200 Subject: dba34a: corrected identified in map file --- cppuhelper/source/cc5_solaris_sparc.map | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cppuhelper') diff --git a/cppuhelper/source/cc5_solaris_sparc.map b/cppuhelper/source/cc5_solaris_sparc.map index 2202d6fa8..6d4c9e6ce 100755 --- a/cppuhelper/source/cc5_solaris_sparc.map +++ b/cppuhelper/source/cc5_solaris_sparc.map @@ -390,6 +390,6 @@ UDK_3.7 { # OOo 3.3 UDK_3.8 { # OOo 3.4 global: - __1cEcppuSOPropertySetHelperbDsetDependentFastPropertyValue6MklrknDcomDsunEstarDunoDAny__v_; + __1cEcppuSOPropertySetHelperbDsetDependentFastPropertyValue6MlrknDcomDsunEstarDunoDAny__v_; } UDK_3.7; -- cgit v1.2.3 From ac8cf22ffe3b2e18e349babc59a287f395acee13 Mon Sep 17 00:00:00 2001 From: Vladimir Glazunov Date: Thu, 11 Nov 2010 15:41:49 +0100 Subject: #i10000# added export for x64 --- cppuhelper/source/gcc3.map | 1 + 1 file changed, 1 insertion(+) (limited to 'cppuhelper') diff --git a/cppuhelper/source/gcc3.map b/cppuhelper/source/gcc3.map index f842854e2..7bc84fbab 100644 --- a/cppuhelper/source/gcc3.map +++ b/cppuhelper/source/gcc3.map @@ -386,5 +386,6 @@ UDK_3.6 { # OOo 3.3 UDK_3.7 { # OOo 3.4 global: _ZN4cppu18OPropertySetHelper29setDependentFastPropertyValueElRKN3com3sun4star3uno3AnyE; + _ZN4cppu18OPropertySetHelper29setDependentFastPropertyValueEiRKN3com3sun4star3uno3AnyE; } UDK_3.6; -- cgit v1.2.3