uitest/uitest/test.py | 5 + vcl/CppunitTest_vcl_idle_task.mk | 37 +++++++++++++ vcl/Module_vcl.mk | 1 vcl/qa/cppunit/IdleTask.cxx | 110 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 153 insertions(+)
New commits: commit a3546f2a045c84580e6c8b263d5f834a0058a732 Author: Neil Roberts <[email protected]> AuthorDate: Tue Nov 18 14:26:42 2025 +0100 Commit: Noel Grandin <[email protected]> CommitDate: Thu Nov 20 09:04:17 2025 +0100 uitest: Wait for idle when calling close_doc for a database doc The UITest calls .uno:CloseDoc at the end of create_db_in_start_center. This seems to end up triggering some race condition which makes any tests using it occasionally fail. I think what happens is this: If the document is modified when CloseDoc is called then instead of immediately closing the document it posts an event to the event queue which causes the main thread to start closing the document asynchronously. In the meantime the UI test thread continues and executes Desktop::terminate. Both the main thread and the UI test thread end up calling XFrameImpl::setComponent simultaneously. The main thread does this with the solar mutex held because the event handling code locks the mutex during the entire process. The UI test thread doesn’t hold it, but parts of XFrameImpl::setComponent require the lock so it blocks straight away. The first part of XFrameImpl::setComponent sets the controller to the new one and then starts disposing the old one. The disposing implementation for OApplicationController temporarily releases the solar mutex while it does the potentially time consuming process of disconnecting from the database (see the SolarMutexReleaser in ODriverDelegator::shutdownConnection). This causes the UI test thread to unblock and continue executing XFrameImpl::setComponent. Presumably it bypasses trying to dispose the controller because that will have already been set to null by the main thread. It will therefore jump straight to disposing the old component window. This invokes VCLXWindow::dispose which ends up disposing the ODataView with the solar mutex locked. Once it is finished and the main thread has finished disconnecting from the database it will be able to reacquire the solar mutex and continue invoking OApplicationController::disposing. The next step calls attachFrame which in turn calls attachFrame on the ODataView. However the data view has been disposed in the meantime which makes it call a method on a null pointer because m_pAccel is now null. This will end up throwing an uncaught std::system_error. I tried working around this one problem by checking whether the ODataView has already been disposed in ODataView::attachFrame. However this then just triggers a different problem where Base tries to close the database connection in one thread while ODriverDelegator::flushConnections is running in another and it also ends up throwing a std::system_error. Seeing as there seems to be a nest of bugs in this area this patch just makes it wait for CloseDoc to finish before continuing the rest of the unit test. If we want to fix the problems later we could make some further unit tests that explicitly execute CloseDoc without waiting. Change-Id: Ie531ada3d740e9e777d2b73ee162d4818fb6c477 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194181 Tested-by: Jenkins Reviewed-by: Noel Grandin <[email protected]> diff --git a/uitest/uitest/test.py b/uitest/uitest/test.py index 915386dd7975..c8d1c3433570 100644 --- a/uitest/uitest/test.py +++ b/uitest/uitest/test.py @@ -233,6 +233,11 @@ class UITest(object): yield component finally: self.close_doc() + # If the document has been modified then close_doc will end up being run + # asynchronously. The Base code seems to have a few race conditions so to avoid that + # let’s wait to make sure the close completes before continuing + xToolkit = self._xContext.ServiceManager.createInstance('com.sun.star.awt.Toolkit') + xToolkit.waitUntilAllIdlesDispatched() def close_dialog_through_button(self, button, dialog=None): if dialog is None: commit 4e47309999229a3082cc6a8d938d0f0868a12d58 Author: Neil Roberts <[email protected]> AuthorDate: Wed Nov 19 11:22:35 2025 +0100 Commit: Noel Grandin <[email protected]> CommitDate: Thu Nov 20 09:04:05 2025 +0100 Add a unit test for IdleTask::waitUntilIdleDispatched The unit test ensures that any pending tasks have actually completed even if they temporarily release the solar mutex. Without ChangeId Ib5c6efe31dfb5e1a5039b43702e23bf0104cd403 the function would return as soon as the solar mutex is released instead. Change-Id: I23f8f365d3e8198d61f805f6b5d15b6ce098055b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194194 Tested-by: Jenkins Reviewed-by: Noel Grandin <[email protected]> diff --git a/vcl/CppunitTest_vcl_idle_task.mk b/vcl/CppunitTest_vcl_idle_task.mk new file mode 100644 index 000000000000..80a3425b3bd2 --- /dev/null +++ b/vcl/CppunitTest_vcl_idle_task.mk @@ -0,0 +1,37 @@ +# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t; fill-column: 100 -*- +# +# 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/. +# + +$(eval $(call gb_CppunitTest_CppunitTest,vcl_idle_task)) + +$(eval $(call gb_CppunitTest_set_include,vcl_idle_task,\ + $$(INCLUDE) \ + -I$(SRCDIR)/vcl/inc \ +)) + +$(eval $(call gb_CppunitTest_add_exception_objects,vcl_idle_task, \ + vcl/qa/cppunit/IdleTask \ +)) + +$(eval $(call gb_CppunitTest_use_libraries,vcl_idle_task, \ + comphelper \ + sal \ + salhelper \ + vcl \ +)) + +$(eval $(call gb_CppunitTest_use_sdk_api,vcl_idle_task)) + +$(eval $(call gb_CppunitTest_use_ure,vcl_idle_task)) +$(eval $(call gb_CppunitTest_use_vcl,vcl_idle_task)) + +$(eval $(call gb_CppunitTest_use_rdb,vcl_idle_task,services)) + +$(eval $(call gb_CppunitTest_use_configuration,vcl_idle_task)) + +# vim: set noet sw=4 ts=4: diff --git a/vcl/Module_vcl.mk b/vcl/Module_vcl.mk index 517bdf723d39..ed9247d58d4e 100644 --- a/vcl/Module_vcl.mk +++ b/vcl/Module_vcl.mk @@ -255,6 +255,7 @@ $(eval $(call gb_Module_add_check_targets,vcl,\ CppunitTest_vcl_filter_igif \ CppunitTest_vcl_unit_conversion \ CppunitTest_vcl_localize_digits \ + CppunitTest_vcl_idle_task \ )) ifeq ($(USING_X11),TRUE) diff --git a/vcl/qa/cppunit/IdleTask.cxx b/vcl/qa/cppunit/IdleTask.cxx new file mode 100644 index 000000000000..874544df6638 --- /dev/null +++ b/vcl/qa/cppunit/IdleTask.cxx @@ -0,0 +1,110 @@ +/* -*- 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 <cppunit/TestFixture.h> +#include <cppunit/extensions/HelperMacros.h> +#include <cppunit/plugin/TestPlugIn.h> + +#include <vcl/svapp.hxx> +#include <vcl/idletask.hxx> +#include <salhelper/thread.hxx> + +// This tests that IdleThread::waitUntilIdleDispatched actually waits until the main thread is idle +// and all event processing is actually completed instead of just waiting until all the events have +// just been popped off the queue. + +namespace +{ +void eventCallback(void*, void* pData) +{ + bool* pDone = static_cast<bool*>(pData); + + // Release the solar mutex for a little while. This shouldn’t + // cause waitUntilIdleDispatched to return + { + SolarMutexReleaser aRelease; + std::this_thread::sleep_for(std::chrono::seconds(10)); + } + + *pDone = true; +} + +struct OtherThread : public salhelper::Thread +{ + bool m_bDone; + + OtherThread() + : salhelper::Thread("IdleTask test") + , m_bDone(false) + { + } + +protected: + void execute() override; + +private: + DECL_LINK(setDone, void*, void); +}; + +void OtherThread::execute() +{ + bool bEventCallbackDone = false; + + Application::PostUserEvent(LINK_NONMEMBER(nullptr, eventCallback), &bEventCallbackDone); + + // Sleep for a little bit to make sure the main thread picks up + // the pending event before this thread does + std::this_thread::sleep_for(std::chrono::seconds(5)); + + // Wait for idle. This should wait for eventCallback to actually + // complete, not just that the main loop has started executing it. + ::IdleTask::waitUntilIdleDispatched(); + + // If this is false then eventCallback hasn’t actually completed + CPPUNIT_ASSERT(bEventCallbackDone); + + // Post another event to wake up the main thread + Application::PostUserEvent(LINK(this, OtherThread, setDone), nullptr); +} + +IMPL_LINK_NOARG(OtherThread, setDone, void*, void) { m_bDone = true; } + +CPPUNIT_TEST_FIXTURE(CppUnit::TestFixture, IdleTaskOtherThread) +{ + CPPUNIT_ASSERT(Application::IsMainThread()); + + rtl::Reference<OtherThread> pOtherThread(new OtherThread); + + pOtherThread->launch(); + + SolarMutexGuard aGuard; + + while (!pOtherThread->m_bDone) + Application::Yield(); + + pOtherThread->join(); +} + +CPPUNIT_TEST_FIXTURE(CppUnit::TestFixture, IdleTaskMainThread) +{ + CPPUNIT_ASSERT(Application::IsMainThread()); + + bool bEventCallbackDone = false; + + Application::PostUserEvent(LINK_NONMEMBER(nullptr, eventCallback), &bEventCallbackDone); + + ::IdleTask::waitUntilIdleDispatched(); + + CPPUNIT_ASSERT(bEventCallbackDone); +} +} // namespace + +CPPUNIT_PLUGIN_IMPLEMENT(); + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
