[Libreoffice-commits] core.git: framework/source postprocess/qa scaddins/source sc/qa sc/source sd/qa svx/source test/source

Noel Grandin (via logerrit) logerrit at kemper.freedesktop.org
Thu Jul 30 08:50:07 UTC 2020


 framework/source/services/desktop.cxx       |    2 -
 postprocess/qa/services.cxx                 |    5 ++-
 sc/qa/unit/bugfix-test.cxx                  |    9 ------
 sc/source/ui/view/tabvwsh4.cxx              |    6 ++--
 scaddins/source/analysis/analysis.component |    2 +
 scaddins/source/analysis/analysis.cxx       |   19 ++++++++++++-
 scaddins/source/analysis/analysis.hxx       |   12 ++++++--
 sd/qa/unit/uimpress.cxx                     |    6 ----
 svx/source/gengal/gengal.cxx                |    3 ++
 test/source/setupvcl.cxx                    |   39 +++++++++++++++++++---------
 10 files changed, 67 insertions(+), 36 deletions(-)

New commits:
commit 6e35794cad555485955c3b43593497dcdbf29840
Author:     Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Mon Jul 27 11:06:41 2020 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Thu Jul 30 10:49:27 2020 +0200

    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 at collabora.co.uk>

diff --git a/framework/source/services/desktop.cxx b/framework/source/services/desktop.cxx
index d3ebf627b86c..8f9f3f0bac85 100644
--- a/framework/source/services/desktop.cxx
+++ b/framework/source/services/desktop.cxx
@@ -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;
diff --git a/postprocess/qa/services.cxx b/postprocess/qa/services.cxx
index 282197acab52..8441e92c1f00 100644
--- a/postprocess/qa/services.cxx
+++ b/postprocess/qa/services.cxx
@@ -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();
     }
 }
 
diff --git a/sc/qa/unit/bugfix-test.cxx b/sc/qa/unit/bugfix-test.cxx
index bf09da9872f2..62e356ddb6d2 100644
--- a/sc/qa/unit/bugfix-test.cxx
+++ b/sc/qa/unit/bugfix-test.cxx
@@ -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();
diff --git a/sc/source/ui/view/tabvwsh4.cxx b/sc/source/ui/view/tabvwsh4.cxx
index a07e05d70739..60d1573f6dd6 100644
--- a/sc/source/ui/view/tabvwsh4.cxx
+++ b/sc/source/ui/view/tabvwsh4.cxx
@@ -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();
diff --git a/scaddins/source/analysis/analysis.component b/scaddins/source/analysis/analysis.component
index 4f287f727ce8..bda78b4e5ab9 100644
--- a/scaddins/source/analysis/analysis.component
+++ b/scaddins/source/analysis/analysis.component
@@ -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>
diff --git a/scaddins/source/analysis/analysis.cxx b/scaddins/source/analysis/analysis.cxx
index b951eca29fc6..8b423b86331f 100644
--- a/scaddins/source/analysis/analysis.cxx
+++ b/scaddins/source/analysis/analysis.cxx
@@ -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());
 }
diff --git a/scaddins/source/analysis/analysis.hxx b/scaddins/source/analysis/analysis.hxx
index 5b9d90bdf172..dbfe11a9c2c0 100644
--- a/scaddins/source/analysis/analysis.hxx
+++ b/scaddins/source/analysis/analysis.hxx
@@ -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 );
diff --git a/sd/qa/unit/uimpress.cxx b/sd/qa/unit/uimpress.cxx
index cd9a98296ad2..0732f93be8ae 100644
--- a/sd/qa/unit/uimpress.cxx
+++ b/sd/qa/unit/uimpress.cxx
@@ -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);
diff --git a/svx/source/gengal/gengal.cxx b/svx/source/gengal/gengal.cxx
index 572b9e081723..d57174ff8913 100644
--- a/svx/source/gengal/gengal.cxx
+++ b/svx/source/gengal/gengal.cxx
@@ -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();
diff --git a/test/source/setupvcl.cxx b/test/source/setupvcl.cxx
index 56c0a3b3c84c..7e489effb307 100644
--- a/test/source/setupvcl.cxx
+++ b/test/source/setupvcl.cxx
@@ -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;
+
+    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 {
-            config = css::configuration::theDefaultProvider::get(context);
+            xDesktop->terminate();
         } 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);
-    }
+
+    css::uno::Reference<css::lang::XComponent>(
+        context, css::uno::UNO_QUERY_THROW)->dispose();
+
+    comphelper::setProcessServiceFactory(nullptr);
 }
 
 }


More information about the Libreoffice-commits mailing list