Apache OpenOffice (AOO) Bugzilla – Issue 113722
cppuhelper: OSingleFactoryHelper::createInstanceWithArgumentsAndContext may leak if XInitialization is not supported
Last modified: 2017-05-20 09:24:57 UTC
The call stack create the ChartModel is, chartmodelmi.dll!chart::ChartModel::ChartModel(const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> & xContext={...}) Line 108 C++ chartmodelmi.dll!chart::ChartModel::create(const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> & xContext={...}) Line 210 + 0x55 bytes C++ cppuhelper3MSC.dll!cppu::OSingleFactoryHelper::createInstanceEveryTime(const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> & xContext={...}) Line 177 + 0x10 bytes C++ cppuhelper3MSC.dll!cppu::OSingleFactoryHelper::createInstanceWithContext(const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> & xContext={...}) Line 218 + 0x19 bytes C++ cppuhelper3MSC.dll!cppu::OFactoryComponentHelper::createInstanceWithContext(const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> & xContext={...}) Line 502 + 0x11 bytes C++ > cppuhelper3MSC.dll!cppu::OSingleFactoryHelper::createInstanceWithArgumentsAndContext(const com::sun::star::uno::Sequence<com::sun::star::uno::Any> & rArguments={...}, const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> & xContext={...}) Line 226 + 0x16 bytes C++ cppuhelper3MSC.dll!cppu::OFactoryComponentHelper::createInstanceWithArgumentsAndContext(const com::sun::star::uno::Sequence<com::sun::star::uno::Any> & rArguments={...}, const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> & xContext={...}) Line 521 + 0x15 bytes C++ cppuhelper3MSC.dll!cppu::ORegistryFactoryHelper::createInstanceWithArgumentsAndContext(const com::sun::star::uno::Sequence<com::sun::star::uno::Any> & rArguments={...}, const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> & xContext={...}) Line 828 + 0x2c bytes C++ bootstrap.uno.dll!stoc_smgr::OServiceManager::createInstanceWithArgumentsAndContext(const rtl::OUString & rServiceSpecifier={...}, const com::sun::star::uno::Sequence<com::sun::star::uno::Any> & rArguments={...}, const com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> & xContext={...}) Line 1340 + 0x29 bytes C++ bootstrap.uno.dll!stoc_smgr::OServiceManager::createInstanceWithArguments(const rtl::OUString & rServiceSpecifier={...}, const com::sun::star::uno::Sequence<com::sun::star::uno::Any> & rArguments={...}) Line 1396 + 0x25 bytes C++ embobj.dll!CreateDocument(const com::sun::star::uno::Reference<com::sun::star::lang::XMultiServiceFactory> & _rxFactory={...}, const rtl::OUString & _rDocumentServiceName={...}, bool _bEmbeddedScriptSupport=true) Line 216 + 0x37 bytes C++ embobj.dll!OCommonEmbeddedObject::InitNewDocument_Impl() Line 310 + 0x41 bytes C++ embobj.dll!OCommonEmbeddedObject::setPersistentEntry(const com::sun::star::uno::Reference<com::sun::star::embed::XStorage> & xStorage={...}, const rtl::OUString & sEntName={...}, long nEntryConnectionMode=0x00000001, const com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> & lArguments={...}, const com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> & lObjArgs={...}) Line 1097 + 0x12 bytes C++ In above callstack, it calls ... OSingleFactoryHelper::createInstanceWithArgumentsAndContext() ...-> cppuhelper3MSC.dll!cppu::OSingleFactoryHelper::createInstanceWithArgumentsAndContext() ... -> chartmodelmi.dll!chart::ChartModel::ChartModel() Below is the ChartModel Ctor, it creates cyclic reference between ChartModel and ChartModel.m_pImplChartModel. To release it, ChartModel::Dispose() must be called. ChartModel::ChartModel(uno::Reference<uno::XComponentContext > const & xContext) : m_aLifeTimeManager( this, this ) , m_bReadOnly( sal_False ) , m_bModified( sal_False ) , m_nInLoad(0) , m_bUpdateNotificationsPending(false) , m_aControllers( m_aModelMutex ) , m_nControllerLockCount(0) , m_xContext( xContext ) // default visual area is 8 x 7 cm , m_aVisualAreaSize( 8000, 7000 ) { OSL_TRACE( "ChartModel: CTOR called" ); // attention: passing this as reference to ImplChartModel m_pImplChartModel.reset( new impl::ImplChartModel( xContext, this )); // reference } In OSingleFactoryHelper::createInstanceWithArgumentsAndContext(), xInit.is() always fails, OSingleFactoryHelper::createInstanceWithArgumentsAndContext() { Reference< XInterface > xRet( createInstanceWithContext( xContext ) ); Reference< lang::XInitialization > xInit( xRet, UNO_QUERY ); if (xInit.is()) // When creating ChartModel, it always fails!!! { xInit->initialize( rArguments ); } else { ... throw lang::IllegalArgumentException(); // } ... } So in CreateDocument() in persistence.cxx, it will recreate the ChartModel again. To fix this problem, OSingleFactoryHelper::createInstanceWithArgumentsAndContext(), before throwing out lang::IllegalArgumentException, ChartModel::dispose() must be called.
Created attachment 71001 [details] fix code patch file (Based on OOo3.1M11 code)
Should this really create a cycle?
@kr: I don't see where the cycle is when applying the patch. @iha: Please take over. I must admit that I do not fully understand the situation here.
Since ChartModel does't implement the lang::XInitialization UNO interface, then in OSingleFactoryHelper::createInstanceWithArgumentsAndContext() API the created first ChartModel object is always discarded before throwing out the lang::IllegalArgumentException. But as the XComponent pattern defines, before ChartModel object can be finally freed, ChartModel::dispose() must be called. In this case, the first ChartModel object always leaks.
Thanks zhangjfibm for pointing to that problem! The patch works. However I would suggest an additional fix. To avoid the unnecessary duplicate construction of the chart I will implement the XInitialization interface at the ChartModel. Then this leak of course does not occur anymore for the chart, but as the potential leak within SingleFactoryHelper::createInstanceWithArgumentsAndContext can affect arbitrary objects (not only the chart) I keep the code that cleans up the created objects before throwing out. @zhangjfibm, when changing code please avoid to introduce new tabs - use spaces instead. Also it is good style to use the same coding style that is used in the edited file already. For example if brackets are symmetric, do not introduce the asymmetric form. Thanks! Fixed in CWS chart49: http://hg.services.openoffice.org/cws/chart49/rev/d5ef7bc5ed01
Thank you.
zhangjfibm, please verify the issue with CWS chart49. Thanks!
Verified, thank you.
reopen it because wrongly closed.
change state because wrongly closed.
mark it as verified.