basic/inc/sbxbase.hxx | 7 ++--
basic/source/classes/sb.cxx | 55 +++++++++++----------------------------
basic/source/inc/sbintern.hxx | 37 +++++++++++++++++++++++---
basic/source/runtime/basrdll.cxx | 5 +++
basic/source/sbx/sbxbase.cxx | 13 ++++-----
5 files changed, 64 insertions(+), 53 deletions(-)
New commits:
commit 231e1e416c039d1f9724962a89cf0573a3db48a2
Author: Noel Grandin <noelgran...@gmail.com>
AuthorDate: Wed Jul 29 20:41:48 2020 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Thu Jul 30 10:10:22 2020 +0200
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.gran...@collabora.co.uk>
diff --git a/basic/inc/sbxbase.hxx b/basic/inc/sbxbase.hxx
index 269f6029a55a..361dd52bd691 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 1de6c8203426..c9f34e90ae16 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 75e3ede9a6bb..9a0436813f42 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 6da6ed9e2e2d..29cd292e2bdf 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 c80b681c644d..f62949ada7ec 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();
- }
}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
