From 07b399eb91fb7860190504ca0214bdf4d30dfe2a Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Thu, 24 Sep 2020 13:58:44 +0200 Subject: framework: lock the solar mutex in loadComponentFromURL() with OnMainThread Regression from commit 2dc3a6c273cb82506842864481d78df7294debbf (framework: allow loading a component on the main thread, 2018-12-19), which was a forward-port from a 5.4-based vendor branch, where this was (it turns out) just working by accident, but never on master. It can happen that loadComponentFromURL() is invoked on a thread, which does not own the solar mutex. Then once vcl::SolarThreadExecutor::execute() is called, it'll try to release the solar mutex. But SolarMutexReleaser is unsafe: it'll release the mutex even if it is owned by an other thread. To make this a bit more safer, it'll abort in comphelper::SolarMutex::doRelease(), in case the current thread doesn't have the mutex already. Fix the problem by taking the solar mutex in loadComponentFromURL(): this is meant to cause no performance problems, since the actual importers typically start with taking the solar mutex anyway. Taking it earlier would be problematic, since this can be invoked by UNO clients directly. Taking it later in vcl/ would be also unusual: typically vcl just asserts that the solar mutex is locked, doesn't take it itself. Change-Id: I752006a91f16a02254d1b5ac6301100ab282630b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/103264 Reviewed-by: Miklos Vajna Tested-by: Jenkins --- framework/qa/cppunit/services.cxx | 133 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 framework/qa/cppunit/services.cxx (limited to 'framework/qa') diff --git a/framework/qa/cppunit/services.cxx b/framework/qa/cppunit/services.cxx new file mode 100644 index 000000000000..93d05ff65ca5 --- /dev/null +++ b/framework/qa/cppunit/services.cxx @@ -0,0 +1,133 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +using namespace ::com::sun::star; + +namespace +{ +/// Covers framework/source/services/ fixes. +class Test : public test::BootstrapFixture, public unotest::MacrosTest +{ +protected: + uno::Reference mxComponent; + +public: + void setUp() override; + void tearDown() override; + uno::Reference& getComponent() { return mxComponent; } +}; + +void Test::setUp() +{ + test::BootstrapFixture::setUp(); + + mxDesktop.set(frame::Desktop::create(mxComponentContext)); +} + +void Test::tearDown() +{ + if (mxComponent.is()) + mxComponent->dispose(); + + test::BootstrapFixture::tearDown(); +} + +/// Invokes XFrameImpl::loadComponentFromURL() on a thread. +class TestThread : public salhelper::Thread +{ + uno::Reference mxComponentLoader; + uno::Reference& mrComponent; + +public: + TestThread(const uno::Reference& xComponentLoader, + uno::Reference& rComponent); + void execute() override; +}; + +TestThread::TestThread(const uno::Reference& xComponentLoader, + uno::Reference& rComponent) + : salhelper::Thread("TestThread") + , mxComponentLoader(xComponentLoader) + , mrComponent(rComponent) +{ +} + +void TestThread::execute() +{ + sal_Int32 nSearchFlags = frame::FrameSearchFlag::AUTO; + uno::Sequence aArguments = { + comphelper::makePropertyValue("OnMainThread", true), + }; + // Note how this is invoking loadComponentFromURL() on a frame, not on the desktop, as usual. + mrComponent = mxComponentLoader->loadComponentFromURL("private:factory/swriter", "_self", + nSearchFlags, aArguments); +} + +CPPUNIT_TEST_FIXTURE(Test, testLoadComponentFromURL) +{ + // Without the accompanying fix in place, this test would have failed with: + // thread 1: comphelper::SolarMutex::doRelease end: m_nCount is 1 + // thread 2: vcl::SolarThreadExecutor::execute: before SolarMutexReleaser ctor + // thread 2: comphelper::SolarMutex::doRelease start: m_nCount is 1, bUnlockAll is 1 + // thread 2: comphelper::SolarMutex::doRelease: failed IsCurrentThread() check, will abort + // Notice how thread 2 attempts to release the solar mutex while thread 1 holds it. + + // Create a default window, so by the time the thread would post a user event, it doesn't need + // the solar mutex to process a SendMessageW() call on Windows. + VclPtrInstance xWindow(nullptr, WB_APP | WB_STDWORK); + // Variable is not used, it holds the default window. + (void)xWindow; + + rtl::Reference xThread; + { + // Start the thread that will load the component, but hold the solar mutex for now, so we + // can see if it blocks. + SolarMutexGuard guard; + uno::Reference xFrame = mxDesktop->findFrame("_blank", /*nSearchFlags=*/0); + uno::Reference xComponentLoader(xFrame, uno::UNO_QUERY); + xThread = new TestThread(xComponentLoader, getComponent()); + xThread->launch(); + // If loadComponentFromURL() doesn't lock the solar mutex, the test will abort here. + osl::Thread::wait(std::chrono::seconds(1)); + } + { + // Now release the solar mutex, so the thread can post the task on the main loop. + SolarMutexReleaser releaser; + osl::Thread::wait(std::chrono::seconds(1)); + } + { + // Spin the main loop. + SolarMutexGuard guard; + Scheduler::ProcessEventsToIdle(); + } + { + // Stop the thread. + SolarMutexReleaser releaser; + xThread->join(); + } +} +} + +CPPUNIT_PLUGIN_IMPLEMENT(); + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ -- cgit