configure.ac | 45 ++++++++++++++++ vcl/CustomTarget_kde4_moc.mk | 3 - vcl/unx/kde4/KDE4FilePicker.cxx | 22 ++++---- vcl/unx/kde4/KDESalInstance.cxx | 2 vcl/unx/kde4/KDEXLib.cxx | 23 ++++---- vcl/unx/kde4/KDEXLib.hxx | 4 - vcl/unx/kde4/tst_exclude_posted_events.hxx | 70 ++++++++++++++++++++++++++ vcl/unx/kde4/tst_exclude_socket_notifiers.hxx | 5 + 8 files changed, 150 insertions(+), 24 deletions(-)
New commits: commit 508337db0c53caa5fb43ef26f781df159497a482 Author: LuboÅ¡ LuÅák <l.lu...@collabora.com> Date: Fri Apr 25 14:06:57 2014 +0200 add better info on the Qt patches needed for KFileDialog Change-Id: I1902f962ac03b171c5e7a45d9c8e59450b04418f diff --git a/configure.ac b/configure.ac index 507678a..d377325 100644 --- a/configure.ac +++ b/configure.ac @@ -11243,6 +11243,8 @@ int main(int argc, char **argv) { AC_DEFINE(KDE_HAVE_GLIB,1) KDE_GLIB_CFLAGS=$(printf '%s' "$KDE_GLIB_CFLAGS" | sed -e "s/-I/${ISYSTEM?}/g") + qt4_fix_warning= + AC_LANG_PUSH([C++]) save_CXXFLAGS=$CXXFLAGS CXXFLAGS="$CXXFLAGS $KDE4_CFLAGS" @@ -11269,7 +11271,12 @@ int main(int argc, char *argv[]) AC_MSG_RESULT([yes]) ],[ AC_MSG_RESULT([no]) - AC_MSG_WARN([native KDE4 file pickers will be disabled at runtime - fix your Qt4 library!]) + AC_MSG_WARN([native KDE4 file pickers will be disabled at runtime]) + if test -z "$qt4_fix_warning"; then + add_warning "native KDE4 file pickers will be disabled at runtime, Qt4 fixes needed" + fi + qt4_fix_warning=1 + add_warning " https://bugreports.qt-project.org/browse/QTBUG-37380 (needed)" ]) # Remove meta object data @@ -11296,12 +11303,21 @@ int main(int argc, char *argv[]) AC_MSG_RESULT([yes]) ],[ AC_MSG_RESULT([no]) - AC_MSG_WARN([native KDE4 file pickers will be disabled at runtime - fix your Qt4 library!]) + AC_MSG_WARN([native KDE4 file pickers will be disabled at runtime]) + if test -z "$qt4_fix_warning"; then + add_warning "native KDE4 file pickers will be disabled at runtime, Qt4 fixes needed" + fi + qt4_fix_warning=1 + add_warning " https://bugreports.qt-project.org/browse/QTBUG-34614 (needed)" ]) # Remove meta object data rm -f "${TSTBASE}."* + if test -n "$qt4_fix_warning"; then + add_warning " https://bugreports.qt-project.org/browse/QTBUG-38585 (recommended)" + fi + LIBS=$save_LIBS CXXFLAGS=$save_CXXFLAGS AC_LANG_POP([C++]) diff --git a/vcl/unx/kde4/KDEXLib.cxx b/vcl/unx/kde4/KDEXLib.cxx index f6c15b0..faa1ff1 100644 --- a/vcl/unx/kde4/KDEXLib.cxx +++ b/vcl/unx/kde4/KDEXLib.cxx @@ -192,9 +192,9 @@ void KDEXLib::Init() // that will release SolarMutex when waiting for more events. // Moreover there are bugs in Qt event loop code that allow QClipboard recursing because the event // loop processes also events that it should not at that point, so no dialogs in that case either. + // https://bugreports.qt-project.org/browse/QTBUG-37380 + // https://bugreports.qt-project.org/browse/QTBUG-34614 if (m_isGlibEventLoopType && (0 == tst_processEventsExcludeSocket()) && tst_excludePostedEvents() == 0 ) - // See http://bugreports.qt.nokia.com/browse/QTBUG-37380 - // https://bugreports.qt-project.org/browse/QTBUG-34614 m_allowKdeDialogs = true; #endif commit 65a3622148ea67744c9c1fc18c2b8d48e5f1c79f Author: LuboÅ¡ LuÅák <l.lu...@collabora.com> Date: Fri Apr 25 13:08:23 2014 +0200 disable KFileDialog usage if QClipboard can recurse Change-Id: If23a365b96c1634c2f8940f6ece973089dc3151f diff --git a/configure.ac b/configure.ac index d77291e..507678a 100644 --- a/configure.ac +++ b/configure.ac @@ -11275,6 +11275,33 @@ int main(int argc, char *argv[]) # Remove meta object data rm -f "${TSTBASE}."* + AC_MSG_CHECKING([whether Qt avoids QClipboard recursion caused by posted events]) + + # Prepare meta object data + TSTBASE="tst_exclude_posted_events" + TSTMOC="${SRC_ROOT}/vcl/unx/kde4/${TSTBASE}" + ln -fs "${TSTMOC}.hxx" + $MOC4 "${TSTBASE}.hxx" -o "${TSTBASE}.moc" + + AC_RUN_IFELSE([AC_LANG_SOURCE([[ +#include "tst_exclude_posted_events.moc" + +int main(int argc, char *argv[]) +{ + QCoreApplication app(argc, argv); + exit(tst_excludePostedEvents()); + return 0; +} + ]])],[ + AC_MSG_RESULT([yes]) + ],[ + AC_MSG_RESULT([no]) + AC_MSG_WARN([native KDE4 file pickers will be disabled at runtime - fix your Qt4 library!]) + ]) + + # Remove meta object data + rm -f "${TSTBASE}."* + LIBS=$save_LIBS CXXFLAGS=$save_CXXFLAGS AC_LANG_POP([C++]) diff --git a/vcl/CustomTarget_kde4_moc.mk b/vcl/CustomTarget_kde4_moc.mk index 9e41754..16d1561 100644 --- a/vcl/CustomTarget_kde4_moc.mk +++ b/vcl/CustomTarget_kde4_moc.mk @@ -12,7 +12,8 @@ $(eval $(call gb_CustomTarget_CustomTarget,vcl/unx/kde4)) $(call gb_CustomTarget_get_target,vcl/unx/kde4) : \ $(call gb_CustomTarget_get_workdir,vcl/unx/kde4)/KDEXLib.moc \ $(call gb_CustomTarget_get_workdir,vcl/unx/kde4)/KDE4FilePicker.moc \ - $(call gb_CustomTarget_get_workdir,vcl/unx/kde4)/tst_exclude_socket_notifiers.moc + $(call gb_CustomTarget_get_workdir,vcl/unx/kde4)/tst_exclude_socket_notifiers.moc \ + $(call gb_CustomTarget_get_workdir,vcl/unx/kde4)/tst_exclude_posted_events.moc $(call gb_CustomTarget_get_workdir,vcl/unx/kde4)/%.moc : \ $(SRCDIR)/vcl/unx/kde4/%.hxx \ diff --git a/vcl/unx/kde4/KDESalInstance.cxx b/vcl/unx/kde4/KDESalInstance.cxx index 023d790..094cd20 100644 --- a/vcl/unx/kde4/KDESalInstance.cxx +++ b/vcl/unx/kde4/KDESalInstance.cxx @@ -35,7 +35,7 @@ uno::Reference< ui::dialogs::XFilePicker2 > KDESalInstance::createFilePicker( const uno::Reference< uno::XComponentContext >& xMSF ) { KDEXLib* kdeXLib = static_cast<KDEXLib*>( mpXLib ); - if (kdeXLib->haveQt4SocketExcludeFix()) + if (kdeXLib->allowKdeDialogs()) return uno::Reference< ui::dialogs::XFilePicker2 >( kdeXLib->createFilePicker(xMSF) ); else diff --git a/vcl/unx/kde4/KDEXLib.cxx b/vcl/unx/kde4/KDEXLib.cxx index 24557bc..f6c15b0 100644 --- a/vcl/unx/kde4/KDEXLib.cxx +++ b/vcl/unx/kde4/KDEXLib.cxx @@ -47,13 +47,14 @@ #if KDE_HAVE_GLIB #include "KDE4FilePicker.hxx" #include "tst_exclude_socket_notifiers.moc" +#include "tst_exclude_posted_events.moc" #endif KDEXLib::KDEXLib() : SalXLib(), m_bStartupDone(false), m_pApplication(0), m_pFreeCmdLineArgs(0), m_pAppCmdLineArgs(0), m_nFakeCmdLineArgs( 0 ), m_frameWidth( -1 ), m_isGlibEventLoopType(false), - m_haveQt4SocketExcludeFix(false) + m_allowKdeDialogs(false) { // the timers created here means they belong to the main thread. // As the timeoutTimer runs the LO event queue, which may block on a dialog, @@ -187,9 +188,14 @@ void KDEXLib::Init() #if KDE_HAVE_GLIB m_isGlibEventLoopType = QAbstractEventDispatcher::instance()->inherits( "QEventDispatcherGlib" ); - if (m_isGlibEventLoopType && (0 == tst_processEventsExcludeSocket())) + // Using KDE dialogs (and their nested event loops) works only with a proper event loop integration + // that will release SolarMutex when waiting for more events. + // Moreover there are bugs in Qt event loop code that allow QClipboard recursing because the event + // loop processes also events that it should not at that point, so no dialogs in that case either. + if (m_isGlibEventLoopType && (0 == tst_processEventsExcludeSocket()) && tst_excludePostedEvents() == 0 ) // See http://bugreports.qt.nokia.com/browse/QTBUG-37380 - m_haveQt4SocketExcludeFix = true; + // https://bugreports.qt-project.org/browse/QTBUG-34614 + m_allowKdeDialogs = true; #endif setupEventLoop(); @@ -238,7 +244,7 @@ void KDEXLib::setupEventLoop() { old_gpoll = g_main_context_get_poll_func( NULL ); g_main_context_set_poll_func( NULL, gpoll_wrapper ); - if( m_haveQt4SocketExcludeFix ) + if( m_allowKdeDialogs ) m_pApplication->clipboard()->setProperty( "useEventLoopWhenWaiting", true ); return; } diff --git a/vcl/unx/kde4/KDEXLib.hxx b/vcl/unx/kde4/KDEXLib.hxx index e45543d..1f2a2dd 100644 --- a/vcl/unx/kde4/KDEXLib.hxx +++ b/vcl/unx/kde4/KDEXLib.hxx @@ -53,7 +53,7 @@ class KDEXLib : public QObject, public SalXLib QTimer userEventTimer; int m_frameWidth; bool m_isGlibEventLoopType; - bool m_haveQt4SocketExcludeFix; + bool m_allowKdeDialogs; private: void setupEventLoop(); @@ -88,7 +88,7 @@ class KDEXLib : public QObject, public SalXLib virtual void PostUserEvent() SAL_OVERRIDE; void doStartup(); - bool haveQt4SocketExcludeFix() { return m_haveQt4SocketExcludeFix; } + bool allowKdeDialogs() { return m_allowKdeDialogs; } public Q_SLOTS: com::sun::star::uno::Reference< com::sun::star::ui::dialogs::XFilePicker2 > diff --git a/vcl/unx/kde4/tst_exclude_posted_events.hxx b/vcl/unx/kde4/tst_exclude_posted_events.hxx new file mode 100644 index 0000000..777907c --- /dev/null +++ b/vcl/unx/kde4/tst_exclude_posted_events.hxx @@ -0,0 +1,70 @@ +/* -*- 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/. + * + * This file incorporates work covered by the following license notice: + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed + * with this work for additional information regarding copyright + * ownership. The ASF licenses this file to you under the Apache + * License, Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of + * the License at http://www.apache.org/licenses/LICENSE-2.0 . + * + * This code is based on the SocketEventsTester from the Qt4 test suite. + */ + +#pragma once + +#include <qcoreapplication.h> +#include <qeventloop.h> +#include <qtimer.h> + +const QEvent::Type eventType = QEvent::User; + +class Test + : public QObject +{ + Q_OBJECT + public: + Test(); + virtual bool event( QEvent* e ); + bool processed; +}; + +Test::Test() + : processed( false ) +{ +} + +bool Test::event( QEvent* e ) +{ + if( e->type() == eventType ) + processed = true; + return QObject::event( e ); +} + +#define QVERIFY(a) \ + if (!a) return 1; + +static int tst_excludePostedEvents() +{ + Test test; + QCoreApplication::postEvent( &test, new QEvent( eventType )); + QEventLoop loop; + QTimer::singleShot(200, &loop, SLOT(quit())); + loop.processEvents(QEventLoop::ExcludeUserInputEvents + | QEventLoop::ExcludeSocketNotifiers +// | QEventLoop::WaitForMoreEvents + | QEventLoop::X11ExcludeTimers); + QVERIFY( !test.processed ); + QTimer::singleShot(200, &loop, SLOT(quit())); + loop.exec(); + QVERIFY( test.processed ); + return 0; +} commit e809aa1e916e0f6d1a849d0374f59ef9619b1db7 Author: LuboÅ¡ LuÅák <l.lu...@collabora.com> Date: Fri Apr 25 13:05:43 2014 +0200 fix Qt4 QSocketNotifier configure check When built as a part of the configure check, this doesn't know SAL_OVERRIDE. Change-Id: I1420dd50efdd6666f246884f286a3f29e0b56a2c diff --git a/vcl/unx/kde4/tst_exclude_socket_notifiers.hxx b/vcl/unx/kde4/tst_exclude_socket_notifiers.hxx index 01f01b9..297cdf2 100644 --- a/vcl/unx/kde4/tst_exclude_socket_notifiers.hxx +++ b/vcl/unx/kde4/tst_exclude_socket_notifiers.hxx @@ -28,6 +28,11 @@ #include <QtNetwork/qtcpserver.h> #include <QtNetwork/qtcpsocket.h> +// This is also used by a configure check. +#ifndef SAL_OVERRIDE +#define SAL_OVERRIDE +#endif + class SocketEventsTester: public QObject { Q_OBJECT commit 474ad6b0e2fb18370be9d228456a2abbfc15bad2 Author: LuboÅ¡ LuÅák <l.lu...@collabora.com> Date: Fri Apr 25 08:57:42 2014 +0200 make sure KFileDialog does not leave the SolarMutex released Change-Id: I806bf5fe1cd1871de499ceeeadf36de539e9d637 diff --git a/vcl/unx/kde4/KDE4FilePicker.cxx b/vcl/unx/kde4/KDE4FilePicker.cxx index b92d86f..5f121cc 100644 --- a/vcl/unx/kde4/KDE4FilePicker.cxx +++ b/vcl/unx/kde4/KDE4FilePicker.cxx @@ -256,19 +256,17 @@ sal_Int16 SAL_CALL KDE4FilePicker::execute() _dialog->setFilter(_filter); _dialog->filterWidget()->setEditable(false); - // At this point, SolarMutex is held. Opening the KDE file dialog here - // can lead to QClipboard asking for clipboard contents. If LO core - // is the owner of the clipboard content, this will block for 5 seconds - // and timeout, since the clipboard thread will not be able to acquire - // SolarMutex and thus won't be able to respond. If the event loops + // KFileDialog intergration requires using event loop with QClipboard. + // Opening the KDE file dialog here can lead to QClipboard + // asking for clipboard contents. If LO core is the owner of the clipboard + // content, without event loop use this will block for 5 seconds and timeout, + // since the clipboard thread will not be able to acquire SolarMutex + // and thus won't be able to respond. If the event loops // are properly integrated and QClipboard can use a nested event loop - // (see the KDE VCL plug), then this won't happen, but otherwise - // simply release the SolarMutex here. The KDE file dialog does not - // call back to the core, so this should be safe (and if it does, - // SolarMutex will need to be re-acquired). - long mutexrelease = 0; - if( !qApp->clipboard()->property( "useEventLoopWhenWaiting" ).toBool()) - mutexrelease = Application::ReleaseSolarMutex(); + // (see the KDE VCL plug), then this won't happen. + // We cannot simply release SolarMutex here, because the event loop started + // by the file dialog would also call back to LO code. + assert( qApp->clipboard()->property( "useEventLoopWhenWaiting" ).toBool() == true ); //block and wait for user input int result = _dialog->exec(); // HACK: KFileDialog uses KConfig("kdeglobals") for saving some settings @@ -276,8 +274,6 @@ sal_Int16 SAL_CALL KDE4FilePicker::execute() // (which is probably a KDE bug), so force reading the new configuration, // otherwise the next opening of the dialog would use the old settings. KGlobal::config()->reparseConfiguration(); - if( !qApp->clipboard()->property( "useEventLoopWhenWaiting" ).toBool()) - Application::AcquireSolarMutex( mutexrelease ); if( result == KFileDialog::Accepted) return ExecutableDialogResults::OK; commit 2cd8a1e0f1e81efd15979953d7f274ab8a6806d6 Author: LuboÅ¡ LuÅák <l.lu...@collabora.com> Date: Fri Mar 28 15:09:13 2014 +0100 Revert "Rewrite Qt4 based nested yield mutex locking." In a dbgutil build LO aborts on an assertion failure the moment KFileDialog is open. It's definitely on okay to release SolarMutex just like that, since the Qt event loop is integrated with LO's, it'll call back to LO code without the mutex held. This reverts commit 13a34f4c6307d1bd2443cbf3fbd83bfdd8cdbafb. Conflicts: vcl/unx/kde4/KDE4FilePicker.cxx vcl/unx/kde4/KDEXLib.cxx Change-Id: Idfa27fbb4728b529c37c25f710ea208fdaa4455c diff --git a/vcl/unx/kde4/KDE4FilePicker.cxx b/vcl/unx/kde4/KDE4FilePicker.cxx index 969fd2c..b92d86f 100644 --- a/vcl/unx/kde4/KDE4FilePicker.cxx +++ b/vcl/unx/kde4/KDE4FilePicker.cxx @@ -256,20 +256,28 @@ sal_Int16 SAL_CALL KDE4FilePicker::execute() _dialog->setFilter(_filter); _dialog->filterWidget()->setEditable(false); - // We're entering a nested loop. - int result; - { - // Release the yield mutex to prevent deadlocks. - SalYieldMutexReleaser aReleaser; - result = _dialog->exec(); - } - + // At this point, SolarMutex is held. Opening the KDE file dialog here + // can lead to QClipboard asking for clipboard contents. If LO core + // is the owner of the clipboard content, this will block for 5 seconds + // and timeout, since the clipboard thread will not be able to acquire + // SolarMutex and thus won't be able to respond. If the event loops + // are properly integrated and QClipboard can use a nested event loop + // (see the KDE VCL plug), then this won't happen, but otherwise + // simply release the SolarMutex here. The KDE file dialog does not + // call back to the core, so this should be safe (and if it does, + // SolarMutex will need to be re-acquired). + long mutexrelease = 0; + if( !qApp->clipboard()->property( "useEventLoopWhenWaiting" ).toBool()) + mutexrelease = Application::ReleaseSolarMutex(); + //block and wait for user input + int result = _dialog->exec(); // HACK: KFileDialog uses KConfig("kdeglobals") for saving some settings // (such as the auto-extension flag), but that doesn't update KGlobal::config() // (which is probably a KDE bug), so force reading the new configuration, // otherwise the next opening of the dialog would use the old settings. KGlobal::config()->reparseConfiguration(); - + if( !qApp->clipboard()->property( "useEventLoopWhenWaiting" ).toBool()) + Application::AcquireSolarMutex( mutexrelease ); if( result == KFileDialog::Accepted) return ExecutableDialogResults::OK; diff --git a/vcl/unx/kde4/KDEXLib.cxx b/vcl/unx/kde4/KDEXLib.cxx index e4900a7..24557bc 100644 --- a/vcl/unx/kde4/KDEXLib.cxx +++ b/vcl/unx/kde4/KDEXLib.cxx @@ -286,16 +286,13 @@ void KDEXLib::Yield( bool bWait, bool bHandleAllCurrentEvents ) } return SalXLib::Yield( bWait, bHandleAllCurrentEvents ); } - // if we are the main thread (which is where the event processing is done), // good, just do it if( qApp->thread() == QThread::currentThread()) processYield( bWait, bHandleAllCurrentEvents ); else - { - // we were called from another thread; - // release the yield lock to prevent deadlock. - SalYieldMutexReleaser aReleaser; + { // if this deadlocks, event processing needs to go into a separate thread + // or some other solution needs to be found Q_EMIT processYieldSignal( bWait, bHandleAllCurrentEvents ); } }
_______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits