[Libreoffice-commits] core.git: 5 commits - configure.ac vcl/CustomTarget_kde4_moc.mk vcl/unx
Luboš Luňák
l.lunak at collabora.com
Fri Apr 25 05:18:44 PDT 2014
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.lunak at 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.lunak at 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.lunak at 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.lunak at 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.lunak at 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 );
}
}
More information about the Libreoffice-commits
mailing list