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

5 files changed, 149 insertions, 9 deletions

diff --git a/cppuhelper/inc/cppuhelper/propshlp.hxx b/cppuhelper/inc/cppuhelper/propshlp.hxx
index 98103e58185b..82bbd2bdcdbe 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
+
+        <p>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.</p>
+
+        <p>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.</p>
+
+        <p><strong>Note</strong>: The method will <em>not</em> invoke veto listeners for the property.</p>
+
+        <p><strong>Note</strong>: 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.</p>
+    */
+    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<const Impl> m_pReserved;
+    const std::auto_ptr<Impl> m_pReserved;
 
 private:
     OPropertySetHelper( const OPropertySetHelper & ) SAL_THROW( () );
     OPropertySetHelper &    operator = ( const OPropertySetHelper & ) SAL_THROW( () );
 
+    /** notifies the given changes in property's values, <em>plus</em> 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 d43ea690e11a..9fa108558b89 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 59ab83e34e82..178a345d4a0b 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 6d5a491ab925..7069276e2baa 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 fe455781a1a7..84638494f2a7 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 );
     }
 }
 
@@ -521,6 +578,38 @@ Any OPropertySetHelper::getFastPropertyValue( sal_Int32 nHandle )
 }
 
 //--------------------------------------------------------------------------
+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
 (
     sal_Int32 * pnHandles,
@@ -803,7 +892,7 @@ void OPropertySetHelper::setFastPropertyValues(
         }
 
         // fire change events
-        fire( pHandles, pConvertedValues, pOldValues, n, sal_False );
+        impl_fireAll( pHandles, pConvertedValues, pOldValues, n );
     }
     catch( ... )
     {