Em sexta-feira, 4 de fevereiro de 2011, às 05:24:26, Dawit A escreveu: > > In the meantime, I've been discussing with Rémi about the issue and he's > > not budging from his position that libraries shoul > > never use Unix signals. To be > > honest, he's right: Unix signals are meant to be used centrally only. > > But we need SIGCHLD to be notified that a child exited, so we need a > > compromise between apps and libraries. > > That is especially true for QProcess. It is used in several place > within kdelibs. If any of those code paths are hit from an app like > VLC, the same issue will manifest itself as well. I am still wondering > whether or not the QtCreator freeze I get to this day is somehow > related to this issue even though I doubt QtCreator does any signal > blocking...
Qt workaround patch. I will most definitely not add this to 4.7. I will show to other engineers in the office and we'll consider 4.8. http://qt.pastebin.com/U6dF8kzd -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Senior Product Manager - Nokia, Qt Development Frameworks PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
>From 3fcabe791c4d6b939e6d43b7fbd4d8b4bb792e0c Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macie...@nokia.com>
Date: Fri, 4 Feb 2011 17:39:09 +0100
Subject: [PATCH] Make QProcess on Unix use threads, not SIGCHLD.
This makes each process started spawn a new thread, which
will block on waitpid(2). There's no thread reusal or a
thread pool (we need one thread running per subprocess being
watched).
We really need instead the kernel to give us a file descriptor
that does the job of this thread.
I don't like this solution, as it's wasteful of resources. I'm
working on a solution that won't start the thread unless the
stderr and stdout pipes close (if they were open in the first place)
but the subprocess didn't exit. Those kinds of processes are
rare and I would accept the resource usage.
Reviewed-By: no one yet
---
src/corelib/io/qprocess.cpp | 8 +-
src/corelib/io/qprocess_p.h | 3 +
src/corelib/io/qprocess_unix.cpp | 249 ++++++--------------------------------
3 files changed, 48 insertions(+), 212 deletions(-)
diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp
index 1ce1256..8575f0d 100644
--- a/src/corelib/io/qprocess.cpp
+++ b/src/corelib/io/qprocess.cpp
@@ -757,6 +757,7 @@ QProcessPrivate::QProcessPrivate()
#endif // Q_WS_WIN
#ifdef Q_OS_UNIX
serial = 0;
+ unixWaiterThread = 0;
#endif
#ifdef Q_OS_SYMBIAN
symbianProcess = NULL;
@@ -826,14 +827,15 @@ void QProcessPrivate::cleanup()
qDeleteInEventHandler(notifier);
notifier = 0;
}
+#ifdef Q_OS_UNIX
+ serial = 0;
+ cleanUpWaiterThread();
+#endif
destroyPipe(stdoutChannel.pipe);
destroyPipe(stderrChannel.pipe);
destroyPipe(stdinChannel.pipe);
destroyPipe(childStartedPipe);
destroyPipe(deathPipe);
-#ifdef Q_OS_UNIX
- serial = 0;
-#endif
#ifdef Q_OS_SYMBIAN
if (symbianProcess) {
symbianProcess->Close();
diff --git a/src/corelib/io/qprocess_p.h b/src/corelib/io/qprocess_p.h
index be4f2a0..1b317be 100644
--- a/src/corelib/io/qprocess_p.h
+++ b/src/corelib/io/qprocess_p.h
@@ -76,6 +76,7 @@ QT_BEGIN_NAMESPACE
class QSocketNotifier;
class QWindowsPipeWriter;
class QWinEventNotifier;
+struct QUnixWaiterThread;
class QTimer;
#if defined(Q_OS_SYMBIAN)
class RProcess;
@@ -205,6 +206,7 @@ public:
void startProcess();
#if defined(Q_OS_UNIX) && !defined(Q_OS_SYMBIAN)
void execChild(const char *workingDirectory, char **path, char **argv, char **envp);
+ void cleanUpWaiterThread();
#endif
bool processStarted();
void terminateProcess();
@@ -225,6 +227,7 @@ public:
QProcess::ExitStatus exitStatus;
bool crashed;
#ifdef Q_OS_UNIX
+ QUnixWaiterThread *unixWaiterThread;
int serial;
#endif
diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 5f53300..b249538 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -105,6 +105,7 @@ QT_END_NAMESPACE
#include <errno.h>
#include <stdlib.h>
#include <string.h>
+#include <pthread.h>
QT_BEGIN_NAMESPACE
@@ -119,21 +120,6 @@ static inline char *strdup(const char *data)
}
#endif
-static int qt_qprocess_deadChild_pipe[2];
-static struct sigaction qt_sa_old_sigchld_handler;
-static void qt_sa_sigchld_handler(int signum)
-{
- qt_safe_write(qt_qprocess_deadChild_pipe[1], "", 1);
-#if defined (QPROCESS_DEBUG)
- fprintf(stderr, "*** SIGCHLD\n");
-#endif
-
- // load it as volatile
- void (*oldAction)(int) = ((volatile struct sigaction *)&qt_sa_old_sigchld_handler)->sa_handler;
- if (oldAction && oldAction != SIG_IGN)
- oldAction(signum);
-}
-
static inline void add_fd(int &nfds, int fd, fd_set *fdset)
{
FD_SET(fd, fdset);
@@ -141,187 +127,24 @@ static inline void add_fd(int &nfds, int fd, fd_set *fdset)
nfds = fd;
}
-struct QProcessInfo {
- QProcess *process;
+struct QUnixWaiterThread {
+ QProcessPrivate *process;
+ pthread_t waitThread;
int deathPipe;
int exitResult;
pid_t pid;
- int serialNumber;
};
-class QProcessManager : public QThread
-{
- Q_OBJECT
-public:
- QProcessManager();
- ~QProcessManager();
-
- void run();
- void catchDeadChildren();
- void add(pid_t pid, QProcess *process);
- void remove(QProcess *process);
- void lock();
- void unlock();
-
-private:
- QMutex mutex;
- QMap<int, QProcessInfo *> children;
-};
-
-
-Q_GLOBAL_STATIC(QMutex, processManagerGlobalMutex)
-
-static QProcessManager *processManager() {
- // The constructor of QProcessManager should be called only once
- // so we cannot use Q_GLOBAL_STATIC directly for QProcessManager
- QMutex *mutex = processManagerGlobalMutex();
- QMutexLocker locker(mutex);
- static QProcessManager processManager;
- return &processManager;
-}
-
-QProcessManager::QProcessManager()
-{
-#if defined (QPROCESS_DEBUG)
- qDebug() << "QProcessManager::QProcessManager()";
-#endif
- // initialize the dead child pipe and make it non-blocking. in the
- // extremely unlikely event that the pipe fills up, we do not under any
- // circumstances want to block.
- qt_safe_pipe(qt_qprocess_deadChild_pipe, O_NONBLOCK);
-
- // set up the SIGCHLD handler, which writes a single byte to the dead
- // child pipe every time a child dies.
- struct sigaction action;
- memset(&action, 0, sizeof(action));
- action.sa_handler = qt_sa_sigchld_handler;
- action.sa_flags = SA_NOCLDSTOP;
- ::sigaction(SIGCHLD, &action, &qt_sa_old_sigchld_handler);
-}
-
-QProcessManager::~QProcessManager()
-{
- // notify the thread that we're shutting down.
- qt_safe_write(qt_qprocess_deadChild_pipe[1], "@", 1);
- qt_safe_close(qt_qprocess_deadChild_pipe[1]);
- wait();
-
- // on certain unixes, closing the reading end of the pipe will cause
- // select in run() to block forever, rather than return with EBADF.
- qt_safe_close(qt_qprocess_deadChild_pipe[0]);
-
- qt_qprocess_deadChild_pipe[0] = -1;
- qt_qprocess_deadChild_pipe[1] = -1;
-
- qDeleteAll(children.values());
- children.clear();
-
- struct sigaction currentAction;
- ::sigaction(SIGCHLD, 0, ¤tAction);
- if (currentAction.sa_handler != qt_sa_sigchld_handler) {
- ::sigaction(SIGCHLD, &qt_sa_old_sigchld_handler, 0);
- }
-}
-
-void QProcessManager::run()
-{
- forever {
- fd_set readset;
- FD_ZERO(&readset);
- FD_SET(qt_qprocess_deadChild_pipe[0], &readset);
-
-#if defined (QPROCESS_DEBUG)
- qDebug() << "QProcessManager::run() waiting for children to die";
-#endif
-
- // block forever, or until activity is detected on the dead child
- // pipe. the only other peers are the SIGCHLD signal handler, and the
- // QProcessManager destructor.
- int nselect = select(qt_qprocess_deadChild_pipe[0] + 1, &readset, 0, 0, 0);
- if (nselect < 0) {
- if (errno == EINTR)
- continue;
- break;
- }
-
- // empty only one byte from the pipe, even though several SIGCHLD
- // signals may have been delivered in the meantime, to avoid race
- // conditions.
- char c;
- if (qt_safe_read(qt_qprocess_deadChild_pipe[0], &c, 1) < 0 || c == '@')
- break;
-
- // catch any and all children that we can.
- catchDeadChildren();
- }
-}
-
-void QProcessManager::catchDeadChildren()
+extern "C" {
+void *waitThreadFunction(void *arg)
{
- QMutexLocker locker(&mutex);
-
- // try to catch all children whose pid we have registered, and whose
- // deathPipe is still valid (i.e, we have not already notified it).
- QMap<int, QProcessInfo *>::Iterator it = children.begin();
- while (it != children.end()) {
- // notify all children that they may have died. they need to run
- // waitpid() in their own thread.
- QProcessInfo *info = it.value();
- qt_safe_write(info->deathPipe, "", 1);
-
-#if defined (QPROCESS_DEBUG)
- qDebug() << "QProcessManager::run() sending death notice to" << info->process;
-#endif
- ++it;
- }
-}
-
-static QBasicAtomicInt idCounter = Q_BASIC_ATOMIC_INITIALIZER(1);
-
-void QProcessManager::add(pid_t pid, QProcess *process)
-{
-#if defined (QPROCESS_DEBUG)
- qDebug() << "QProcessManager::add() adding pid" << pid << "process" << process;
-#endif
-
- // insert a new info structure for this process
- QProcessInfo *info = new QProcessInfo;
- info->process = process;
- info->deathPipe = process->d_func()->deathPipe[1];
- info->exitResult = 0;
- info->pid = pid;
-
- int serial = idCounter.fetchAndAddRelaxed(1);
- process->d_func()->serial = serial;
- children.insert(serial, info);
-}
-
-void QProcessManager::remove(QProcess *process)
-{
- QMutexLocker locker(&mutex);
-
- int serial = process->d_func()->serial;
- QProcessInfo *info = children.value(serial);
- if (!info)
- return;
-
-#if defined (QPROCESS_DEBUG)
- qDebug() << "QProcessManager::remove() removing pid" << info->pid << "process" << info->process;
-#endif
-
- children.remove(serial);
- delete info;
-}
-
-void QProcessManager::lock()
-{
- mutex.lock();
-}
-
-void QProcessManager::unlock()
-{
- mutex.unlock();
+ // this function must be fully cancellable!
+ QUnixWaiterThread *info = static_cast<QUnixWaiterThread *>(arg);
+ quintptr result = qt_safe_waitpid(info->pid, &info->exitResult, 0);
+ qt_safe_write(info->deathPipe, "", 1);
+ return (void*)result;
}
+} // extern "C"
static void qt_create_pipe(int *pipe)
{
@@ -519,8 +342,6 @@ void QProcessPrivate::startProcess()
qDebug("QProcessPrivate::startProcess()");
#endif
- processManager()->start();
-
// Initialize pipes
if (!createChannel(stdinChannel) ||
!createChannel(stdoutChannel) ||
@@ -625,7 +446,6 @@ void QProcessPrivate::startProcess()
}
// Start the process manager, and fork off the child process.
- processManager()->lock();
pid_t childPid = qt_fork();
int lastForkErrno = errno;
if (childPid != 0) {
@@ -646,7 +466,6 @@ void QProcessPrivate::startProcess()
#if defined (QPROCESS_DEBUG)
qDebug("qt_fork failed: %s", qt_error_string(lastForkErrno));
#endif
- processManager()->unlock();
q->setProcessState(QProcess::NotRunning);
processError = QProcess::FailedToStart;
q->setErrorString(QProcess::tr("Resource error (fork failure): %1").arg(qt_error_string(lastForkErrno)));
@@ -663,9 +482,19 @@ void QProcessPrivate::startProcess()
// Register the child. In the mean time, we can get a SIGCHLD, so we need
// to keep the lock held to avoid a race to catch the child.
- processManager()->add(childPid, q);
pid = Q_PID(childPid);
- processManager()->unlock();
+ unixWaiterThread = new QUnixWaiterThread;
+ unixWaiterThread->process = this;
+ unixWaiterThread->pid = childPid;
+ unixWaiterThread->deathPipe = deathPipe[1];
+ {
+ // start the waiter thread
+ pthread_attr_t thattr;
+ pthread_attr_init(&thattr);
+ pthread_attr_setstacksize(&thattr, PTHREAD_STACK_MIN);
+ pthread_create(&unixWaiterThread->waitThread, &thattr, waitThreadFunction, unixWaiterThread);
+ pthread_attr_destroy(&thattr);
+ }
// parent
// close the ends we don't use and make all pipes non-blocking
@@ -760,6 +589,17 @@ void QProcessPrivate::execChild(const char *workingDir, char **path, char **argv
childStartedPipe[1] = -1;
}
+void QProcessPrivate::cleanUpWaiterThread()
+{
+ if (unixWaiterThread) {
+ void *arg;
+ pthread_cancel(unixWaiterThread->waitThread);
+ pthread_join(unixWaiterThread->waitThread, &arg);
+ delete unixWaiterThread;
+ unixWaiterThread = 0;
+ }
+}
+
bool QProcessPrivate::processStarted()
{
ushort buf[errorBufferMax];
@@ -1134,30 +974,23 @@ bool QProcessPrivate::waitForWrite(int msecs)
void QProcessPrivate::findExitCode()
{
- Q_Q(QProcess);
- processManager()->remove(q);
+ // nothing to do
}
bool QProcessPrivate::waitForDeadChild()
{
- Q_Q(QProcess);
-
// read a byte from the death pipe
char c;
qt_safe_read(deathPipe[0], &c, 1);
// check if our process is dead
- int exitStatus;
- if (qt_safe_waitpid(pid_t(pid), &exitStatus, WNOHANG) > 0) {
- processManager()->remove(q);
- crashed = !WIFEXITED(exitStatus);
- exitCode = WEXITSTATUS(exitStatus);
+ crashed = !WIFEXITED(unixWaiterThread->exitResult);
+ exitCode = WEXITSTATUS(unixWaiterThread->exitResult);
#if defined QPROCESS_DEBUG
qDebug() << "QProcessPrivate::waitForDeadChild() dead with exitCode"
<< exitCode << ", crashed?" << crashed;
#endif
- return true;
- }
+ return true;
#if defined QPROCESS_DEBUG
qDebug() << "QProcessPrivate::waitForDeadChild() not dead!";
#endif
@@ -1170,8 +1003,6 @@ void QProcessPrivate::_q_notified()
bool QProcessPrivate::startDetached(const QString &program, const QStringList &arguments, const QString &workingDirectory, qint64 *pid)
{
- processManager()->start();
-
QByteArray encodedWorkingDirectory = QFile::encodeName(workingDirectory);
// To catch the startup of the child
@@ -1284,7 +1115,7 @@ bool QProcessPrivate::startDetached(const QString &program, const QStringList &a
void QProcessPrivate::initializeProcessManager()
{
- (void) processManager();
+ // Unix no longer has a process manager
}
QT_END_NAMESPACE
--
1.7.3.2.431.ge2f5c
signature.asc
Description: This is a digitally signed message part.
