fix shutdown crash in basic
another change I am working on slightly tweaks the shutdown ordering
and exposes this problem where two classes both think they own
the same object.
Change-Id: I7477cf7eda5b5729ee3861cb4a1be43bb34d9ea6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99724
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
diff --git a/basic/inc/sbxbase.hxx b/basic/inc/sbxbase.hxx
index 269f602..361dd52 100644
--- a/basic/inc/sbxbase.hxx
+++ b/basic/inc/sbxbase.hxx
@@ -38,9 +38,8 @@ struct SbxAppData
{
ErrCode eErrCode; // Error code
SbxVariableRef m_aGlobErr; // Global error object
std::vector<std::unique_ptr<SbxFactory>>
m_Factories;
tools::SvRef<SvRefBase> mrImplRepository;
std::vector<SbxFactory*> m_Factories; // these are owned by
tools::SvRef<SvRefBase> mrImplRepository;
// Pointer to Format()-Command helper class
std::unique_ptr<SbxBasicFormater> pBasicFormater;
@@ -55,6 +54,8 @@ struct SbxAppData
};
SbxAppData& GetSbxData_Impl();
/** returns true if the SbxAppData is still valid, used to check if we are in shutdown. */
bool IsSbxData_Impl();
#endif // INCLUDED_BASIC_INC_SBXBASE_HXX
diff --git a/basic/source/classes/sb.cxx b/basic/source/classes/sb.cxx
index 1de6c820..c9f34e9 100644
--- a/basic/source/classes/sb.cxx
+++ b/basic/source/classes/sb.cxx
@@ -463,14 +463,6 @@ SbxObject* SbiFactory::CreateObject( const OUString& rClass )
}
// Factory class to create OLE objects
class SbOLEFactory : public SbxFactory
{
public:
virtual SbxBase* Create( sal_uInt16 nSbxId, sal_uInt32 ) override;
virtual SbxObject* CreateObject( const OUString& ) override;
};
SbxBase* SbOLEFactory::Create( sal_uInt16, sal_uInt32 )
{
// Not supported
@@ -486,13 +478,6 @@ SbxObject* SbOLEFactory::CreateObject( const OUString& rClassName )
// SbFormFactory, show user forms by: dim as new <user form name>
class SbFormFactory : public SbxFactory
{
public:
virtual SbxBase* Create( sal_uInt16 nSbxId, sal_uInt32 ) override;
virtual SbxObject* CreateObject( const OUString& ) override;
};
SbxBase* SbFormFactory::Create( sal_uInt16, sal_uInt32 )
{
// Not supported
@@ -587,14 +572,6 @@ SbxObject* cloneTypeObjectImpl( const SbxObject& rTypeObj )
return pRet;
}
// Factory class to create user defined objects (type command)
class SbTypeFactory : public SbxFactory
{
public:
virtual SbxBase* Create( sal_uInt16 nSbxId, sal_uInt32 ) override;
virtual SbxObject* CreateObject( const OUString& ) override;
};
SbxBase* SbTypeFactory::Create( sal_uInt16, sal_uInt32 )
{
// Not supported
@@ -922,14 +899,14 @@ StarBASIC::StarBASIC( StarBASIC* p, bool bIsDocBasic )
{
GetSbData()->pSbFac.reset( new SbiFactory );
AddFactory( GetSbData()->pSbFac.get() );
GetSbData()->pTypeFac = new SbTypeFactory;
AddFactory( GetSbData()->pTypeFac );
GetSbData()->pClassFac = new SbClassFactory;
AddFactory( GetSbData()->pClassFac );
GetSbData()->pOLEFac = new SbOLEFactory;
AddFactory( GetSbData()->pOLEFac );
GetSbData()->pFormFac = new SbFormFactory;
AddFactory( GetSbData()->pFormFac );
GetSbData()->pTypeFac.reset(new SbTypeFactory);
AddFactory( GetSbData()->pTypeFac.get() );
GetSbData()->pClassFac.reset(new SbClassFactory);
AddFactory( GetSbData()->pClassFac.get() );
GetSbData()->pOLEFac.reset(new SbOLEFactory);
AddFactory( GetSbData()->pOLEFac.get() );
GetSbData()->pFormFac.reset(new SbFormFactory);
AddFactory( GetSbData()->pFormFac.get() );
GetSbData()->pUnoFac.reset( new SbUnoFactory );
AddFactory( GetSbData()->pUnoFac.get() );
}
@@ -963,14 +940,14 @@ StarBASIC::~StarBASIC()
GetSbData()->pSbFac.reset();
RemoveFactory( GetSbData()->pUnoFac.get() );
GetSbData()->pUnoFac.reset();
RemoveFactory( GetSbData()->pTypeFac );
delete GetSbData()->pTypeFac; GetSbData()->pTypeFac = nullptr;
RemoveFactory( GetSbData()->pClassFac );
delete GetSbData()->pClassFac; GetSbData()->pClassFac = nullptr;
RemoveFactory( GetSbData()->pOLEFac );
delete GetSbData()->pOLEFac; GetSbData()->pOLEFac = nullptr;
RemoveFactory( GetSbData()->pFormFac );
delete GetSbData()->pFormFac; GetSbData()->pFormFac = nullptr;
RemoveFactory( GetSbData()->pTypeFac.get() );
GetSbData()->pTypeFac.reset();
RemoveFactory( GetSbData()->pClassFac.get() );
GetSbData()->pClassFac.reset();
RemoveFactory( GetSbData()->pOLEFac.get() );
GetSbData()->pOLEFac.reset();
RemoveFactory( GetSbData()->pFormFac.get() );
GetSbData()->pFormFac.reset();
if( SbiGlobals::pGlobals )
{
diff --git a/basic/source/inc/sbintern.hxx b/basic/source/inc/sbintern.hxx
index 75e3ede..9a04368 100644
--- a/basic/source/inc/sbintern.hxx
+++ b/basic/source/inc/sbintern.hxx
@@ -78,16 +78,45 @@ public:
SbModule* FindClass( const OUString& rClassName );
};
// Factory class to create user defined objects (type command)
class SbTypeFactory : public SbxFactory
{
public:
virtual SbxBase* Create( sal_uInt16 nSbxId, sal_uInt32 ) override;
virtual SbxObject* CreateObject( const OUString& ) override;
};
class SbFormFactory : public SbxFactory
{
public:
virtual SbxBase* Create( sal_uInt16 nSbxId, sal_uInt32 ) override;
virtual SbxObject* CreateObject( const OUString& ) override;
};
// Factory class to create OLE objects
class SbOLEFactory : public SbxFactory
{
public:
virtual SbxBase* Create( sal_uInt16 nSbxId, sal_uInt32 ) override;
virtual SbxObject* CreateObject( const OUString& ) override;
};
struct SbiGlobals
{
static SbiGlobals* pGlobals;
SbiInstance* pInst; // all active runtime instances
std::unique_ptr<SbiFactory> pSbFac; // StarBASIC-Factory
std::unique_ptr<SbUnoFactory> pUnoFac; // Factory for Uno-Structs at DIM AS NEW
SbTypeFactory* pTypeFac; // Factory for user defined types
SbClassFactory* pClassFac; // Factory for user defined classes (based on class modules)
SbOLEFactory* pOLEFac; // Factory for OLE types
SbFormFactory* pFormFac; // Factory for user forms
std::unique_ptr<SbTypeFactory>
pTypeFac; // Factory for user defined types
std::unique_ptr<SbClassFactory>
pClassFac; // Factory for user defined classes (based on class modules)
std::unique_ptr<SbOLEFactory>
pOLEFac; // Factory for OLE types
std::unique_ptr<SbFormFactory>
pFormFac; // Factory for user forms
SbModule* pMod; // currently active module
SbModule* pCompMod; // currently compiled module
short nInst; // number of BASICs
diff --git a/basic/source/runtime/basrdll.cxx b/basic/source/runtime/basrdll.cxx
index 6da6ed9..29cd292 100644
--- a/basic/source/runtime/basrdll.cxx
+++ b/basic/source/runtime/basrdll.cxx
@@ -123,4 +123,9 @@ SbxAppData& GetSbxData_Impl()
return *BasicDLLImpl::BASIC_DLL->xSbxAppData;
}
bool IsSbxData_Impl()
{
return BasicDLLImpl::BASIC_DLL && BasicDLLImpl::BASIC_DLL->xSbxAppData;
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/basic/source/sbx/sbxbase.cxx b/basic/source/sbx/sbxbase.cxx
index c80b681..f62949a 100644
--- a/basic/source/sbx/sbxbase.cxx
+++ b/basic/source/sbx/sbxbase.cxx
@@ -48,7 +48,9 @@ SbxAppData::~SbxAppData()
pBasicFormater.reset();
// basic manager repository must be destroyed before factories
mrImplRepository.clear();
m_Factories.clear();
// we need to move stuff out otherwise the destruction of the factories
// calls back into SbxBase::RemoveFactory and sees partially destructed data
std::move(m_Factories);
}
SbxBase::SbxBase()
@@ -121,15 +123,12 @@ void SbxBase::AddFactory( SbxFactory* pFac )
void SbxBase::RemoveFactory( SbxFactory const * pFac )
{
if (!IsSbxData_Impl())
return;
SbxAppData& r = GetSbxData_Impl();
auto it = std::find_if(r.m_Factories.begin(), r.m_Factories.end(),
[&pFac](const std::unique_ptr<SbxFactory>& rxFactory) { return rxFactory.get() == pFac; });
auto it = std::find(r.m_Factories.begin(), r.m_Factories.end(), pFac);
if (it != r.m_Factories.end())
{
std::unique_ptr<SbxFactory> tmp(std::move(*it));
r.m_Factories.erase( it );
(void)tmp.release();
}
}
