From 24a231d7a3f54a4a3ac23aed4759bb9b7556c15f Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 18 Sep 2012 13:54:35 +0200 Subject: [PATCH] Re-revert "Delay creation of the process manager" This reverts commit daba2c507ad42c66dafa6a29cffa94e9641e0c58, re-applying commit d9c06bf25210b3d0b31ee6126e57bcb82c292da1, because the change was accidentally brought back in commit eae8fb85997d82ecec0743ba3e470681129bff41. There's a potential deadlock when a QProcess is created while a QCoreApplication is instantiated but never executed, or if the main thread waits() for the child thread. Task-number: QTBUG-27260 Change-Id: I9e0fdc0341b3063de90979377bac35f2a827b260 Reviewed-by: Oswald Buddenhagen --- src/corelib/io/qprocess_unix.cpp | 15 +--- src/corelib/kernel/qcoreapplication.cpp | 18 ++--- src/corelib/kernel/qcoreapplication.h | 1 - src/corelib/kernel/qcoreapplication_p.h | 2 - tests/auto/corelib/io/io.pro | 1 + .../qprocess-noapplication.pro | 5 ++ .../tst_qprocessnoapplication.cpp | 84 ++++++++++++++++++++++ 7 files changed, 98 insertions(+), 28 deletions(-) create mode 100644 tests/auto/corelib/io/qprocess-noapplication/qprocess-noapplication.pro create mode 100644 tests/auto/corelib/io/qprocess-noapplication/tst_qprocessnoapplication.cpp diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp index 1ef7af5..0a928e4 100644 --- a/src/corelib/io/qprocess_unix.cpp +++ b/src/corelib/io/qprocess_unix.cpp @@ -177,7 +177,7 @@ static QProcessManager *processManager() QMutexLocker locker(&processManagerGlobalMutex); if (!processManagerInstance) - QProcessPrivate::initializeProcessManager(); + new QProcessManager; Q_ASSERT(processManagerInstance); return processManagerInstance; @@ -185,9 +185,6 @@ static QProcessManager *processManager() QProcessManager::QProcessManager() { - // can only be called from main thread - Q_ASSERT(!qApp || qApp->thread() == QThread::currentThread()); - #if defined (QPROCESS_DEBUG) qDebug() << "QProcessManager::QProcessManager()"; #endif @@ -1434,15 +1431,7 @@ bool QProcessPrivate::startDetached(const QString &program, const QStringList &a void QProcessPrivate::initializeProcessManager() { - if (qApp && qApp->thread() != QThread::currentThread()) { - // The process manager must be initialized in the main thread - // Note: The call below will re-enter this function, but in the right thread, - // so the else statement below will be executed. - QMetaObject::invokeMethod(qApp, "_q_initializeProcessManager", Qt::BlockingQueuedConnection); - } else { - static QProcessManager processManager; - Q_UNUSED(processManager); - } + (void) processManager(); } QT_END_NAMESPACE diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index 1e4a6f4..df0ffce 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -392,16 +392,6 @@ void QCoreApplicationPrivate::createEventDispatcher() #endif } -void QCoreApplicationPrivate::_q_initializeProcessManager() -{ -#ifndef QT_NO_PROCESS -# ifdef Q_OS_UNIX - QProcessPrivate::initializeProcessManager(); -# endif -#endif -} - - QThread *QCoreApplicationPrivate::theMainThread = 0; QThread *QCoreApplicationPrivate::mainThread() { @@ -625,6 +615,12 @@ void QCoreApplication::init() d->appendApplicationPathToLibraryPaths(); #endif +#if defined(Q_OS_UNIX) && !(defined(QT_NO_PROCESS)) + // Make sure the process manager thread object is created in the main + // thread. + QProcessPrivate::initializeProcessManager(); +#endif + #ifdef QT_EVAL extern void qt_core_eval_init(uint); qt_core_eval_init(d->application_type); @@ -2364,5 +2360,3 @@ void QCoreApplication::setEventDispatcher(QAbstractEventDispatcher *eventDispatc */ QT_END_NAMESPACE - -#include "moc_qcoreapplication.cpp" diff --git a/src/corelib/kernel/qcoreapplication.h b/src/corelib/kernel/qcoreapplication.h index 388877c..622139e 100644 --- a/src/corelib/kernel/qcoreapplication.h +++ b/src/corelib/kernel/qcoreapplication.h @@ -178,7 +178,6 @@ protected: QCoreApplication(QCoreApplicationPrivate &p); private: - Q_PRIVATE_SLOT(d_func(), void _q_initializeProcessManager()) static bool sendSpontaneousEvent(QObject *receiver, QEvent *event); bool notifyInternal(QObject *receiver, QEvent *event); diff --git a/src/corelib/kernel/qcoreapplication_p.h b/src/corelib/kernel/qcoreapplication_p.h index 393ae1c..321f690 100644 --- a/src/corelib/kernel/qcoreapplication_p.h +++ b/src/corelib/kernel/qcoreapplication_p.h @@ -76,8 +76,6 @@ public: bool sendThroughObjectEventFilters(QObject *, QEvent *); bool notify_helper(QObject *, QEvent *); - void _q_initializeProcessManager(); - QString appName() const; virtual void createEventDispatcher(); static void removePostedEvent(QEvent *); diff --git a/tests/auto/corelib/io/io.pro b/tests/auto/corelib/io/io.pro index f6542d9..03b42a2 100644 --- a/tests/auto/corelib/io/io.pro +++ b/tests/auto/corelib/io/io.pro @@ -16,6 +16,7 @@ SUBDIRS=\ qipaddress \ qnodebug \ qprocess \ + qprocess-noapplication \ qprocessenvironment \ qresourceengine \ qsettings \ diff --git a/tests/auto/corelib/io/qprocess-noapplication/qprocess-noapplication.pro b/tests/auto/corelib/io/qprocess-noapplication/qprocess-noapplication.pro new file mode 100644 index 0000000..2f409eb --- /dev/null +++ b/tests/auto/corelib/io/qprocess-noapplication/qprocess-noapplication.pro @@ -0,0 +1,5 @@ +CONFIG += testcase +CONFIG += parallel_test +CONFIG -= app_bundle debug_and_release_target +QT = core testlib +SOURCES = tst_qprocessnoapplication.cpp diff --git a/tests/auto/corelib/io/qprocess-noapplication/tst_qprocessnoapplication.cpp b/tests/auto/corelib/io/qprocess-noapplication/tst_qprocessnoapplication.cpp new file mode 100644 index 0000000..33146ca --- /dev/null +++ b/tests/auto/corelib/io/qprocess-noapplication/tst_qprocessnoapplication.cpp @@ -0,0 +1,84 @@ +/**************************************************************************** +** +** Copyright (C) 2012 Intel Corporation. +** Contact: http://www.qt-project.org/legal +** +** This file is part of the QtCore module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and Digia. For licensing terms and +** conditions see http://qt.digia.com/licensing. For further information +** use the contact form at http://qt.digia.com/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 2.1 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 2.1 requirements +** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Digia gives you certain additional +** rights. These rights are described in the Digia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3.0 as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU General Public License version 3.0 requirements will be +** met: http://www.gnu.org/copyleft/gpl.html. +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#include +#include +#include +#include + +class tst_QProcessNoApplication : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void initializationDeadlock(); +}; + +void tst_QProcessNoApplication::initializationDeadlock() +{ + // see QTBUG-27260 + // QProcess on Unix uses (or used to, at the time of the writing of this test) + // a global class called QProcessManager. + // This class is instantiated (or was) only in the main thread, which meant that + // blocking the main thread while waiting for QProcess could mean a deadlock. + + struct MyThread : public QThread + { + void run() + { + // what we execute does not matter, as long as we try to + // and that the process exits + QProcess::execute("true"); + } + }; + + static char argv0[] = "tst_QProcessNoApplication"; + char *argv[] = { argv0, 0 }; + int argc = 1; + QCoreApplication app(argc, argv); + MyThread thread; + thread.start(); + QVERIFY(thread.wait(10000)); +} + +QTEST_APPLESS_MAIN(tst_QProcessNoApplication) + +#include "tst_qprocessnoapplication.moc" -- 2.7.4