Skip to content

Commit

Permalink
tdf#162696 qt weld: Do GUI things in main thread
Browse files Browse the repository at this point in the history 
Creating or modifying native Qt UI elements needs
to happen in the main thread.

This commit takes care of the obvious cases where
such interaction happens.

Otherwise, the tdf#162696 scenario triggers asserts
like the following with a Qt debug build:

    ASSERT failure in QWidget: "Widgets must be created in the GUI thread.", file /home/michi/development/git/qt5/qtbase/src/widgets/kernel/qwidget.cpp, line 958
    terminate called after throwing an instance of 'com::sun::star::uno::RuntimeException'

    Fatal exception: Signal 6
    Stack:
    #0 sal::backtrace_get(unsigned int) at /home/michi/development/git/libreoffice/sal/osl/unx/backtraceapi.cxx:42
    #1 (anonymous namespace)::printStack(int) at /home/michi/development/git/libreoffice/sal/osl/unx/signal.cxx:289
    #2 (anonymous namespace)::callSystemHandler(int, siginfo_t*, void*) at /home/michi/development/git/libreoffice/sal/osl/unx/signal.cxx:330
    #3 (anonymous namespace)::signalHandlerFunction(int, siginfo_t*, void*) at /home/michi/development/git/libreoffice/sal/osl/unx/signal.cxx:427
    #4 /lib/x86_64-linux-gnu/libc.so.6(+0x3f590) [0x7fdb05455590]
    #5 __pthread_kill_implementation at ./nptl/pthread_kill.c:44 (discriminator 1)
    #6 raise at ./signal/../sysdeps/posix/raise.c:27
    #7 abort at ./stdlib/abort.c:81
    #8 /lib/x86_64-linux-gnu/libstdc++.so.6(+0xa1a3d) [0x7fdb050a1a3d]
    #9 /lib/x86_64-linux-gnu/libstdc++.so.6(+0xb306a) [0x7fdb050b306a]
    #10 std::unexpected() in /lib/x86_64-linux-gnu/libstdc++.so.6
    #11 /home/michi/development/git/qt5/qtbase/lib/libQt6Core.so.6(+0xed562) [0x7fdaf0eed562]
    #12 QMessageLogger::fatal() const at /home/michi/development/git/qt5/qtbase/src/corelib/global/qlogging.cpp:901
    #13 qt_assert_x(char const*, char const*, char const*, int) at /home/michi/development/git/qt5/qtbase/src/corelib/global/qassert.cpp:0
    #14 QWidgetPrivate::init(QWidget*, QFlags<Qt::WindowType>) at /home/michi/development/git/qt5/qtbase/src/widgets/kernel/qwidget.cpp:959
    #15 QWidget::QWidget(QWidgetPrivate&, QWidget*, QFlags<Qt::WindowType>) at /home/michi/development/git/qt5/qtbase/src/widgets/kernel/qwidget.cpp:878
    #16 QDialog::QDialog(QDialogPrivate&, QWidget*, QFlags<Qt::WindowType>) at /home/michi/development/git/qt5/qtbase/src/widgets/dialogs/qdialog.cpp:377
    #17 QMessageBox::QMessageBox(QWidget*) at /home/michi/development/git/qt5/qtbase/src/widgets/dialogs/qmessagebox.cpp:838
    #18 QtInstance::CreateMessageDialog(weld::Widget*, VclMessageType, VclButtonsType, rtl::OUString const&) at /home/michi/development/git/libreoffice/vcl/qt6/../qt5/QtInstance.cxx:825
    #19 Application::CreateMessageDialog(weld::Widget*, VclMessageType, VclButtonsType, rtl::OUString const&, vcl::ILibreOfficeKitNotifier const*) at /home/michi/development/git/libreoffice/vcl/source/window/builder.cxx:224
    #20 dp_gui::DialogHelper::installExtensionWarn(std::basic_string_view<char16_t, std::char_traits<char16_t>>) at /home/michi/development/git/libreoffice/desktop/source/deployment/gui/dp_gui_dialog2.cxx:371
    #21 dp_gui::(anonymous namespace)::ProgressCmdEnv::handle(com::sun::star::uno::Reference<com::sun::star::task::XInteractionRequest> const&) at /home/michi/development/git/libreoffice/desktop/source/deployment/gui/dp_gui_extensioncmdqueue.cxx:477
    #22 dp_misc::interactContinuation(com::sun::star::uno::Any const&, com::sun::star::uno::Type const&, com::sun::star::uno::Reference<com::sun::star::ucb::XCommandEnvironment> const&, bool*, bool*) at /home/michi/development/git/libreoffice/desktop/source/deployment/misc/dp_interact.cxx:114
    #23 dp_manager::ExtensionManager::checkInstall(rtl::OUString const&, com::sun::star::uno::Reference<com::sun::star::ucb::XCommandEnvironment> const&) at /home/michi/development/git/libreoffice/desktop/source/deployment/manager/dp_extensionmanager.cxx:1315
    #24 dp_manager::ExtensionManager::doChecksForAddExtension(com::sun::star::uno::Reference<com::sun::star::deployment::XPackageManager> const&, com::sun::star::uno::Sequence<com::sun::star::beans::NamedValue> const&, com::sun::star::uno::Reference<com::sun::star::deployment::XPackage> const&, com::sun::star::uno::Reference<com::sun::star::task::XAbortChannel> const&, com::sun::star::uno::Reference<com::sun::star::ucb::XCommandEnvironment> const&, com::sun::star::uno::Reference<com::sun::star::deployment::XPackage>&) at /home/michi/development/git/libreoffice/desktop/source/deployment/manager/dp_extensionmanager.cxx:565
    #25 dp_manager::ExtensionManager::addExtension(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::NamedValue> const&, rtl::OUString const&, com::sun::star::uno::Reference<com::sun::star::task::XAbortChannel> const&, com::sun::star::uno::Reference<com::sun::star::ucb::XCommandEnvironment> const&) at /home/michi/development/git/libreoffice/desktop/source/deployment/manager/dp_extensionmanager.cxx:655
    #26 non-virtual thunk to dp_manager::ExtensionManager::addExtension(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::NamedValue> const&, rtl::OUString const&, com::sun::star::uno::Reference<com::sun::star::task::XAbortChannel> const&, com::sun::star::uno::Reference<com::sun::star::ucb::XCommandEnvironment> const&) at /home/michi/development/git/libreoffice/desktop/source/deployment/manager/dp_extensionmanager.cxx:0
    #27 dp_gui::ExtensionCmdQueue::Thread::_addExtension(rtl::Reference<dp_gui::(anonymous namespace)::ProgressCmdEnv> const&, rtl::OUString const&, rtl::OUString const&, bool) at /home/michi/development/git/libreoffice/desktop/source/deployment/gui/dp_gui_extensioncmdqueue.cxx:864
    #28 dp_gui::ExtensionCmdQueue::Thread::execute() at /home/michi/development/git/libreoffice/desktop/source/deployment/gui/dp_gui_extensioncmdqueue.cxx:738
    #29 salhelper::Thread::run() at /home/michi/development/git/libreoffice/salhelper/source/thread.cxx:39
    #30 threadFunc at /home/michi/development/git/libreoffice/include/osl/thread.hxx:190
    #31 osl_thread_start_Impl(void*) at /home/michi/development/git/libreoffice/sal/osl/unx/thread.cxx:245
    #32 start_thread at ./nptl/pthread_create.c:447
    #33 clone3 at ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:80

Change-Id: Ifa84a038fc56f34958cd732caeb9c436b48b3c75
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172642
Tested-by: Jenkins
Reviewed-by: Michael Weghorn <[email protected]>
  • Loading branch information
@michaelweghorn
michaelweghorn committed Aug 30, 2024
1 parent fb1d527 commit 5e4c163
Show file tree
Hide file tree
Showing 6 changed files with 342 additions and 5 deletions.
10 changes: 10 additions & 0 deletions vcl/qt5/QtInstance.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,16 @@ weld::MessageDialog* QtInstance::CreateMessageDialog(weld::Widget* pParent,
VclButtonsType eButtonsType,
const OUString& rPrimaryMessage)
{
SolarMutexGuard g;
if (!IsMainThread())
{
weld::MessageDialog* pDialog;
RunInMainThread([&] {
pDialog = CreateMessageDialog(pParent, eMessageType, eButtonsType, rPrimaryMessage);
});
return pDialog;
}

if (QtData::noWeldedWidgets())
{
return SalInstance::CreateMessageDialog(pParent, eMessageType, eButtonsType,
Expand Down
17 changes: 17 additions & 0 deletions vcl/qt5/QtInstanceButton.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ QtInstanceButton::QtInstanceButton(QPushButton* pButton)

void QtInstanceButton::set_label(const OUString& rText)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
pQtInstance->RunInMainThread([&] { set_label(rText); });
return;
}

assert(m_pButton);
m_pButton->setText(toQString(rText));
}
Expand All @@ -41,6 +49,15 @@ void QtInstanceButton::set_from_icon_name(const OUString& /*rIconName*/)

OUString QtInstanceButton::get_label() const
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
OUString sLabel;
pQtInstance->RunInMainThread([&] { sLabel = get_label(); });
return sLabel;
}

assert(m_pButton);
return toOUString(m_pButton->text());
}
Expand Down
41 changes: 38 additions & 3 deletions vcl/qt5/QtInstanceDialog.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,50 @@ void QtInstanceDialog::SetInstallLOKNotifierHdl(const Link<void*, vcl::ILibreOff
{
}

int QtInstanceDialog::run() { return qtResponseTypeToVclResponseType(m_pDialog->exec()); }
int QtInstanceDialog::run()
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
int nResult = 0;
pQtInstance->RunInMainThread([&] { nResult = run(); });
return nResult;
}

return qtResponseTypeToVclResponseType(m_pDialog->exec());
}

void QtInstanceDialog::response(int) {}

void QtInstanceDialog::add_button(const OUString&, int, const OUString&) {}

void QtInstanceDialog::set_modal(bool bModal) { m_pDialog->setModal(bModal); }
void QtInstanceDialog::set_modal(bool bModal)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
pQtInstance->RunInMainThread([&] { set_modal(bModal); });
return;
}

m_pDialog->setModal(bModal);
}

bool QtInstanceDialog::get_modal() const { return m_pDialog->isModal(); }
bool QtInstanceDialog::get_modal() const
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
bool bModal = false;
pQtInstance->RunInMainThread([&] { bModal = get_modal(); });
return bModal;
}

return m_pDialog->isModal();
}

weld::Button* QtInstanceDialog::weld_widget_for_response(int) { return nullptr; }

Expand Down
112 changes: 111 additions & 1 deletion vcl/qt5/QtInstanceMessageDialog.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,30 +28,72 @@ QtInstanceMessageDialog::QtInstanceMessageDialog(QMessageBox* pMessageDialog)

void QtInstanceMessageDialog::set_primary_text(const rtl::OUString& rText)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
pQtInstance->RunInMainThread([&] { set_primary_text(rText); });
return;
}

m_pMessageDialog->setText(toQString(rText));
}

void QtInstanceMessageDialog::set_secondary_text(const rtl::OUString& rText)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
pQtInstance->RunInMainThread([&] { set_secondary_text(rText); });
return;
}

m_pMessageDialog->setInformativeText(toQString(rText));
}

weld::Container* QtInstanceMessageDialog::weld_message_area() { return nullptr; }

OUString QtInstanceMessageDialog::get_primary_text() const
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
OUString sText;
if (!pQtInstance->IsMainThread())
{
pQtInstance->RunInMainThread([&] { sText = get_primary_text(); });
return sText;
}

assert(m_pMessageDialog);
return toOUString(m_pMessageDialog->text());
}

OUString QtInstanceMessageDialog::get_secondary_text() const
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
OUString sText;
if (!pQtInstance->IsMainThread())
{
pQtInstance->RunInMainThread([&] { sText = get_secondary_text(); });
return sText;
}

assert(m_pMessageDialog);
return toOUString(m_pMessageDialog->informativeText());
}

void QtInstanceMessageDialog::add_button(const OUString& rText, int nResponse, const OUString&)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
pQtInstance->RunInMainThread([&] { add_button(rText, nResponse); });
return;
}

assert(m_pMessageDialog);
QPushButton* pButton = m_pMessageDialog->addButton(vclToQtStringWithAccelerator(rText),
QMessageBox::ButtonRole::ActionRole);
Expand All @@ -60,6 +102,14 @@ void QtInstanceMessageDialog::add_button(const OUString& rText, int nResponse, c

void QtInstanceMessageDialog::set_default_response(int nResponse)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
pQtInstance->RunInMainThread([&] { set_default_response(nResponse); });
return;
}

assert(m_pMessageDialog);

QPushButton* pButton = buttonForResponseCode(nResponse);
Expand All @@ -69,6 +119,15 @@ void QtInstanceMessageDialog::set_default_response(int nResponse)

QtInstanceButton* QtInstanceMessageDialog::weld_widget_for_response(int nResponse)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
QtInstanceButton* pButton;
pQtInstance->RunInMainThread([&] { pButton = weld_widget_for_response(nResponse); });
return pButton;
}

if (QPushButton* pButton = buttonForResponseCode(nResponse))
return new QtInstanceButton(pButton);

Expand All @@ -77,6 +136,15 @@ QtInstanceButton* QtInstanceMessageDialog::weld_widget_for_response(int nRespons

int QtInstanceMessageDialog::run()
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
int nRet = 0;
pQtInstance->RunInMainThread([&] { nRet = run(); });
return nRet;
}

m_pMessageDialog->exec();
QAbstractButton* pClickedButton = m_pMessageDialog->clickedButton();
if (!pClickedButton)
Expand All @@ -87,6 +155,15 @@ int QtInstanceMessageDialog::run()
bool QtInstanceMessageDialog::runAsync(const std::shared_ptr<weld::DialogController>& rxOwner,
const std::function<void(sal_Int32)>& func)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
bool bRet = false;
pQtInstance->RunInMainThread([&] { bRet = runAsync(rxOwner, func); });
return bRet;
}

assert(m_pMessageDialog);

m_xRunAsyncDialogController = rxOwner;
Expand All @@ -100,6 +177,15 @@ bool QtInstanceMessageDialog::runAsync(const std::shared_ptr<weld::DialogControl
bool QtInstanceMessageDialog::runAsync(std::shared_ptr<Dialog> const& rxSelf,
const std::function<void(sal_Int32)>& func)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
bool bRet;
pQtInstance->RunInMainThread([&] { bRet = runAsync(rxSelf, func); });
return bRet;
}

assert(m_pMessageDialog);
assert(rxSelf.get() == this);

Expand All @@ -113,12 +199,28 @@ bool QtInstanceMessageDialog::runAsync(std::shared_ptr<Dialog> const& rxSelf,

void QtInstanceMessageDialog::response(int nResponse)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
pQtInstance->RunInMainThread([&] { response(nResponse); });
return;
}

assert(m_pMessageDialog);
m_pMessageDialog->done(nResponse);
}

void QtInstanceMessageDialog::dialogFinished(int nResult)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
pQtInstance->RunInMainThread([&] { dialogFinished(nResult); });
return;
}

assert(m_aRunAsyncFunc);

disconnect(m_pMessageDialog, &QDialog::finished, this,
Expand All @@ -137,7 +239,6 @@ void QtInstanceMessageDialog::dialogFinished(int nResult)
if (QAbstractButton* pClickedButton = m_pMessageDialog->clickedButton())
nRet = pClickedButton->property(PROPERTY_VCL_RESPONSE_CODE).toInt();

SolarMutexGuard g;
aFunc(nRet);

xRunAsyncDialogController.reset();
Expand All @@ -146,6 +247,15 @@ void QtInstanceMessageDialog::dialogFinished(int nResult)

QPushButton* QtInstanceMessageDialog::buttonForResponseCode(int nResponse)
{
SolarMutexGuard g;
QtInstance* pQtInstance = GetQtInstance();
if (!pQtInstance->IsMainThread())
{
QPushButton* pButton;
pQtInstance->RunInMainThread([&] { pButton = buttonForResponseCode(nResponse); });
return pButton;
}

assert(m_pMessageDialog);

const QList<QAbstractButton*> aButtons = m_pMessageDialog->buttons();
Expand Down
Loading

0 comments on commit 5e4c163

Please sign in to comment.