terminate XDesktop properly in unit tests

So that the UNO constructor work can continue - where we need the
desktop to be disposed properly so that all UNO constructors objects
have their dispose() called, and they can clean up their global state.

We detect this case by changing a SAL_WARN to an assert in
Desktop::disposing()

(*) in ~ScTabViewShell, don't call EnterHandler, because that tries to
create EditEngine's and other stuff, which crashes
(*) Need a fake singleton so that the servicemanager calls dispose()
 on the AnalysAddIn and we can clear the global variable there.

Change-Id: Id13b51e17afc16fcbbc65d64281cdf847e4a58cf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99640
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
This commit is contained in:
Noel Grandin 2020-07-27 11:06:41 +02:00
parent a6e02f6337
commit 6e35794cad
10 changed files with 68 additions and 37 deletions

View file

@ -1040,7 +1040,7 @@ void SAL_CALL Desktop::disposing()
// But if you just ignore the assertion (which happens in unit
// tests for instance in sc/qa/unit) nothing bad happens.
SAL_WARN_IF(!m_bIsShutdown, "fwk.desktop", "Desktop disposed before terminating it");
assert(m_bIsShutdown && "Desktop disposed before terminating it");
{
SolarMutexGuard aWriteLock;

View file

@ -37,6 +37,7 @@
#include <com/sun/star/lang/XServiceInfo.hpp>
#include <com/sun/star/reflection/XServiceConstructorDescription.hpp>
#include <com/sun/star/reflection/XServiceTypeDescription2.hpp>
#include <com/sun/star/frame/XDesktop.hpp>
#include <comphelper/sequence.hxx>
#include <cppuhelper/exc_hlp.hxx>
#include <rtl/strbuf.hxx>
@ -286,7 +287,9 @@ void Test::test() {
}
SolarMutexReleaser rel;
for (auto const & i: comps) {
i->dispose();
// cannot call dispose() on XDesktop before calling terminate()
if (!css::uno::Reference<css::frame::XDesktop>(i, css::uno::UNO_QUERY))
i->dispose();
}
}

View file

@ -30,7 +30,6 @@ public:
ScFiltersTest();
virtual void setUp() override;
virtual void tearDown() override;
void testTdf64229();
void testTdf36933();
@ -178,8 +177,6 @@ void ScFiltersTest::testTdf91979()
int nRowHeight = ScViewData::ToPixel(pDoc->GetRowHeight(0, 0), aViewData.GetPPTY());
CPPUNIT_ASSERT_EQUAL(static_cast<long>((MAXCOL - 1) * nColWidth), aPos.getX());
CPPUNIT_ASSERT_EQUAL(static_cast<long>(10000 * nRowHeight), aPos.getY());
xComponent->dispose();
}
/*
@ -516,12 +513,6 @@ void ScFiltersTest::setUp()
CPPUNIT_ASSERT_MESSAGE("no calc component!", m_xCalcComponent.is());
}
void ScFiltersTest::tearDown()
{
uno::Reference< lang::XComponent >( m_xCalcComponent, UNO_QUERY_THROW )->dispose();
test::BootstrapFixture::tearDown();
}
CPPUNIT_TEST_SUITE_REGISTRATION(ScFiltersTest);
CPPUNIT_PLUGIN_IMPLEMENT();

View file

@ -1755,9 +1755,6 @@ ScTabViewShell::~ScTabViewShell()
if (mpInputHandler)
{
mpInputHandler->SetDocumentDisposing(true);
// We end edit mode, before destroying the input handler and the edit engine
// and before end listening (in order to call ScTabViewShell::KillEditView())
mpInputHandler->EnterHandler();
}
ScDocShell* pDocSh = GetViewData().GetDocShell();
@ -1770,6 +1767,9 @@ ScTabViewShell::~ScTabViewShell()
RemoveSubShell(); // all
SetWindow(nullptr);
// need kill editview or we will touch the editengine after it has been freed by the ScInputHandler
KillEditView(true);
pFontworkBarShell.reset();
pExtrusionBarShell.reset();
pCellShell.reset();

View file

@ -23,5 +23,7 @@
constructor="scaddins_AnalysisAddIn_get_implementation">
<service name="com.sun.star.sheet.AddIn"/>
<service name="com.sun.star.sheet.addin.Analysis"/>
<!-- fake singleton so that the servicemanager calls dispose() on the component and we can free the global var -->
<singleton name="com.sun.star.sheet.addin.theAnalysis"/>
</implementation>
</component>

View file

@ -43,6 +43,10 @@ using namespace ::com::sun::star;
using namespace sca::analysis;
using namespace std;
static osl::Mutex g_InstanceMutex;
static rtl::Reference<AnalysisAddIn> g_Instance;
static bool g_Disposed;
OUString AnalysisAddIn::GetFuncDescrStr(const char** pResId, sal_uInt16 nStrIndex)
{
return AnalysisResId(pResId[nStrIndex - 1]);
@ -59,6 +63,7 @@ void AnalysisAddIn::InitData()
}
AnalysisAddIn::AnalysisAddIn( const uno::Reference< uno::XComponentContext >& xContext ) :
AnalysisAddIn_Base(m_aMutex),
aAnyConv( xContext )
{
}
@ -67,6 +72,14 @@ AnalysisAddIn::~AnalysisAddIn()
{
}
void AnalysisAddIn::dispose()
{
AnalysisAddIn_Base::dispose();
osl::MutexGuard aGuard(g_InstanceMutex);
g_Instance.clear();
g_Disposed = true;
}
sal_Int32 AnalysisAddIn::getDateMode(
const uno::Reference< beans::XPropertySet >& xPropSet,
const uno::Any& rAny )
@ -1056,7 +1069,11 @@ extern "C" SAL_DLLPUBLIC_EXPORT css::uno::XInterface*
scaddins_AnalysisAddIn_get_implementation(
css::uno::XComponentContext* context, css::uno::Sequence<css::uno::Any> const&)
{
static rtl::Reference<AnalysisAddIn> g_Instance(new AnalysisAddIn(context));
osl::MutexGuard aGuard(g_InstanceMutex);
if (g_Disposed)
return nullptr;
if (!g_Instance)
g_Instance.set(new AnalysisAddIn(context));
g_Instance->acquire();
return static_cast<cppu::OWeakObject*>(g_Instance.get());
}

View file

@ -27,7 +27,8 @@
#include <com/sun/star/sheet/addin/XAnalysis.hpp>
#include <com/sun/star/sheet/XCompatibilityNames.hpp>
#include <cppuhelper/implbase.hxx>
#include <cppuhelper/compbase.hxx>
#include <cppuhelper/basemutex.hxx>
#include "analysishelper.hxx"
@ -36,12 +37,14 @@
namespace com::sun::star::lang { class XMultiServiceFactory; }
namespace com::sun::star::sheet { struct LocalizedName; }
class AnalysisAddIn : public cppu::WeakImplHelper<
typedef cppu::WeakComponentImplHelper<
css::sheet::XAddIn,
css::sheet::XCompatibilityNames,
css::sheet::addin::XAnalysis,
css::lang::XServiceName,
css::lang::XServiceInfo >
css::lang::XServiceInfo > AnalysisAddIn_Base;
class AnalysisAddIn : private cppu::BaseMutex, public AnalysisAddIn_Base
{
private:
css::lang::Locale aFuncLoc;
@ -75,6 +78,9 @@ public:
virtual ~AnalysisAddIn() override;
// XComponent
virtual void SAL_CALL dispose() override;
/// @throws css::uno::RuntimeException
/// @throws css::lang::IllegalArgumentException
double FactDouble( sal_Int32 nNum );

View file

@ -31,7 +31,6 @@ namespace {
class Test : public CppUnit::TestFixture {
public:
Test();
virtual ~Test() override;
virtual void setUp() override;
virtual void tearDown() override;
@ -76,11 +75,6 @@ void Test::tearDown()
m_pDoc.reset();
}
Test::~Test()
{
uno::Reference< lang::XComponent >(m_xContext, uno::UNO_QUERY_THROW)->dispose();
}
void Test::testAddPage()
{
SdrPage* pPage = m_pDoc->AllocPage(false);

View file

@ -18,6 +18,7 @@
#include <cppuhelper/bootstrap.hxx>
#include <com/sun/star/lang/XMultiServiceFactory.hpp>
#include <com/sun/star/ucb/UniversalContentBroker.hpp>
#include <com/sun/star/frame/Desktop.hpp>
#include <tools/urlobj.hxx>
#include <vcl/vclmain.hxx>
@ -308,6 +309,8 @@ int GalApp::Main()
void GalApp::DeInit()
{
auto xDesktop = css::frame::Desktop::create(comphelper::getProcessComponentContext());
xDesktop->terminate();
uno::Reference< lang::XComponent >(
comphelper::getProcessComponentContext(),
uno::UNO_QUERY_THROW )-> dispose();

View file

@ -12,6 +12,7 @@
#include <com/sun/star/configuration/theDefaultProvider.hpp>
#include <com/sun/star/lang/XComponent.hpp>
#include <com/sun/star/util/XFlushable.hpp>
#include <com/sun/star/frame/Desktop.hpp>
#include <comphelper/processfactory.hxx>
#include <i18nlangtag/languagetag.hxx>
#include <i18nlangtag/mslangid.hxx>
@ -37,20 +38,34 @@ IMPL_STATIC_LINK_NOARG(Hook, deinitHook, LinkParamNone *, void) {
try {
context = comphelper::getProcessComponentContext();
} catch (css::uno::RuntimeException &) {}
if (context.is()) {
css::uno::Reference<css::lang::XMultiServiceFactory> config;
try {
config = css::configuration::theDefaultProvider::get(context);
} catch (css::uno::DeploymentException &) {}
if (config.is()) {
utl::ConfigManager::storeConfigItems();
css::uno::Reference<css::util::XFlushable>(
config, css::uno::UNO_QUERY_THROW)->flush();
}
css::uno::Reference<css::lang::XComponent>(
context, css::uno::UNO_QUERY_THROW)->dispose();
comphelper::setProcessServiceFactory(nullptr);
if (!context)
return;
css::uno::Reference<css::lang::XMultiServiceFactory> config;
try {
config = css::configuration::theDefaultProvider::get(context);
} catch (css::uno::DeploymentException &) {}
if (config) {
utl::ConfigManager::storeConfigItems();
css::uno::Reference<css::util::XFlushable>(
config, css::uno::UNO_QUERY_THROW)->flush();
}
// the desktop has to be terminate() before it can be dispose()
css::uno::Reference<css::frame::XDesktop> xDesktop;
try {
xDesktop = css::frame::Desktop::create(comphelper::getProcessComponentContext());
} catch (css::uno::DeploymentException &) {}
if (xDesktop)
try {
xDesktop->terminate();
} catch (css::uno::DeploymentException &) {}
css::uno::Reference<css::lang::XComponent>(
context, css::uno::UNO_QUERY_THROW)->dispose();
comphelper::setProcessServiceFactory(nullptr);
}
}