On Sun, Aug 21, 2011 at 12:08:23PM +0200, Lionel Elie Mamane wrote:
> On Sun, Aug 21, 2011 at 01:54:17AM +0200, Lionel Elie Mamane wrote:
>> 11207ae93191fb966676423e6d377c8292a8cf0b
>> Make XPropertSet2 not a child interface of XPropertySet.
>> This is to preserve ABI backward compatibility with
>> cppu::OPropertySetHelper.
>> 80b1e662777100a7dfd80176a2b528880a838167
>> Added XPropertySet2 to allow disabling of change event
>> notifications.
>> From the second (later) commit log message, you intended to preserve
>> ABI, but I get the impressions ABI backwards compatibility is still
>> broken in a different way by the addition of
>> virtual void SAL_CALL enableChangeListenerNotification( sal_Bool bEnable
>> )
>> throw(::com::sun::star::uno::RuntimeException);
>> From what I observe, at least with GNU GCC/g++ on Debian GNU/Linux
>> amd64 (I wouldn't know about MSVC++ and g++ on MS Windows, but
>> possibly it is the same), it seems all the virtual functions that are
>> declared *after* that one in the header are shifted one position in
>> the virtual function table, and that lookups in the virtual function
>> table are by position, not by name/signature.
> Maybe not. It seems my build was hosed, possibly because of wrong
> inter-module dependencies so that a change to
> cppuhelper/propshlp.hxx is not properly propagated everywhere.
> I'm making new tests doing "make clean && make" every time, and I'll
> let you know what I find out.
I confirm that ABI is broken for cppu::OPropertySetHelper with respect
to 3.4 branch.
I observe, additionally to the "wrong function called from new ABI
code to old ABI code that derives (inherits) from
cppu::OPropertySetHelper" that I already mentioned, that the
destructor for cppu::OPropertySetHelper::Impl is called with
non-sensical "this" pointer when (in old ABI code) a reference to a
class that inherits from cppu::OPropertySetHelper goes out of scope
and the object is destroyed. I have the case with postgresql-sdbc
compiled against libreoffice-3-4.
Here's an example trace of cppu::OPropertySetHelper::Impl ctor/dtor
calls:
Impl ctor this='0x2e851a8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e97418'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e862e8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e97598'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e8d0a8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e89128'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e91638'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e91fb8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e92288'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e92558'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e93238'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e91518'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e94298'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e94ac8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e95148'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e95248'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e9a0a8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e9bc48'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f8ebd8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f8d098'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f8db48'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f95438'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f93ec8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f98118'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f96a98'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f9ade8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f99778'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f9db28'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2f9c458'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2fa0108'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2e9a318'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2fa37f8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2fa21d8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2fa6568'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2fa4e98'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2fae6f8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2fc7978'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2fc8398'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl ctor this='0x2fbefa8'; m_handles.size()='0', m_newValues.size()='0',
m_oldValues.size()='0'
Impl dtor this='0x7ffdb09d57c8'; m_handles.size()='148277658',
m_newValues.size()='6148914691236407954',
m_oldValues.size()='-6148914691211804262'
The attached patch makes both these symptoms go away, which is why I
conclude that ABI has changed; I'm not saying this patch should be
applied as-is; rather it should go this way, as mentioned in my first
email:
class OPropertySetHelperFireEventOption : public OPropertySetHelper,
public
::com::sun::star::beans::XPropertySetOptions
{
bool m_bFireEvent;
public:
(...)
virtual void SAL_CALL enableChangeListenerNotification( sal_Bool bEnable )
throw(::com::sun::star::uno::RuntimeException);
(...)
}
So that new code that wants to have the ability to enable/disable
change listener notifications can derive from OPropertySetHelper2
instead, but old code still works.
If people agree that this would be the way to go, I'll prepare a patch
that does that; I'm also looking for a better class name than
OPropertySetHelperFireEventOption :)
--
Lionel
diff --git a/cppuhelper/inc/cppuhelper/propshlp.hxx b/cppuhelper/inc/cppuhelper/propshlp.hxx
index 670ce03..6b4f33e 100644
--- a/cppuhelper/inc/cppuhelper/propshlp.hxx
+++ b/cppuhelper/inc/cppuhelper/propshlp.hxx
@@ -351,8 +351,7 @@ public:
*/
class OPropertySetHelper : public ::com::sun::star::beans::XMultiPropertySet,
public ::com::sun::star::beans::XFastPropertySet,
- public ::com::sun::star::beans::XPropertySet,
- public ::com::sun::star::beans::XPropertySetOption
+ public ::com::sun::star::beans::XPropertySet
{
public:
/**
@@ -505,11 +504,6 @@ public:
const ::com::sun::star::uno::Sequence< ::rtl::OUString >& PropertyNames,
const ::com::sun::star::uno::Reference< ::com::sun::star::beans::XPropertiesChangeListener > & Listener )
throw(::com::sun::star::uno::RuntimeException);
-
- // XPropertySetOption
- virtual void SAL_CALL enableChangeListenerNotification( sal_Bool bEnable )
- throw(::com::sun::star::uno::RuntimeException);
-
/**
The property sequence is created in the call. The interface isn't used after the call.
*/
@@ -637,8 +631,6 @@ protected:
*/
OMultiTypeInterfaceContainerHelperInt32 aVetoableLC;
- bool m_bFireEvent;
-
class Impl;
/** reserved for future use. finally, the future has arrived...
diff --git a/cppuhelper/source/propshlp.cxx b/cppuhelper/source/propshlp.cxx
index 7878062..4f19ba2 100644
--- a/cppuhelper/source/propshlp.cxx
+++ b/cppuhelper/source/propshlp.cxx
@@ -173,7 +173,6 @@ OPropertySetHelper::OPropertySetHelper(
: rBHelper( rBHelper_ ),
aBoundLC( rBHelper_.rMutex ),
aVetoableLC( rBHelper_.rMutex ),
- m_bFireEvent(true),
m_pReserved( new Impl(false, 0) )
{
}
@@ -183,7 +182,6 @@ OPropertySetHelper::OPropertySetHelper(
: rBHelper( rBHelper_ ),
aBoundLC( rBHelper_.rMutex ),
aVetoableLC( rBHelper_.rMutex ),
- m_bFireEvent(true),
m_pReserved( new Impl( bIgnoreRuntimeExceptionsWhileFiring, 0 ) )
{
}
@@ -194,7 +192,6 @@ OPropertySetHelper::OPropertySetHelper(
: rBHelper( rBHelper_ ),
aBoundLC( rBHelper_.rMutex ),
aVetoableLC( rBHelper_.rMutex ),
- m_bFireEvent(true),
m_pReserved(
new Impl( bIgnoreRuntimeExceptionsWhileFiring, i_pFireEvents) )
{
@@ -218,7 +215,6 @@ Any OPropertySetHelper::queryInterface( const ::com::sun::star::uno::Type & rTyp
return ::cppu::queryInterface(
rType,
static_cast< XPropertySet * >( this ),
- static_cast< XPropertySetOption * >( this ),
static_cast< XMultiPropertySet * >( this ),
static_cast< XFastPropertySet * >( this ) );
}
@@ -631,9 +627,6 @@ void OPropertySetHelper::fire
sal_Bool bVetoable
)
{
- if (!m_bFireEvent)
- return;
-
OSL_ENSURE( m_pReserved.get(), "No OPropertySetHelper::Impl" );
if (m_pReserved->m_pFireEvents) {
m_pReserved->m_pFireEvents->fireEvents(
@@ -1037,12 +1030,6 @@ void OPropertySetHelper::firePropertiesChangeEvent(
delete [] pHandles;
}
-void OPropertySetHelper::enableChangeListenerNotification( sal_Bool bEnable )
- throw(::com::sun::star::uno::RuntimeException)
-{
- m_bFireEvent = bEnable;
-}
-
#ifdef xdvnsdfln
// XPropertyState
PropertyState OPropertySetHelper::getPropertyState( const OUString& PropertyName )
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
