[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: The hidden problem with using QProcess/KProcess in kdelibs...
From:       Dawit A <adawit () kde ! org>
Date:       2011-01-22 22:39:52
Message-ID: AANLkTimhyUb83dg-hqmVVFuTyu7_vASoTGYMfJmAwa+o () mail ! gmail ! com
[Download RAW message or body]

On Sat, Jan 22, 2011 at 5:16 PM, Dawit A <adawit@kde.org> wrote:
> On Sat, Jan 22, 2011 at 3:55 PM, Thiago Macieira <thiago@kde.org> wrote:
> > On Saturday, 22 de January de 2011 14:25:07 Dawit A wrote:
> > > Anyhow, this problem is not specific to VLC and can happen to any Qt
> > > application that blocks the SIGCHLD signals from reaching any of the
> > > threads. As such I think some action needs to be taken to address this
> > > issue. We can workaround this issue by unseting and setting the signal
> > > masks in the aforementioned function, but there are many other places
> > > in kdelibs where QProcess is used so perhaps a better resolution is
> > > needed for this ? Perhaps it is QProcess that should be taking care of
> > > this itself ?
> > 
> > I agree with you. QProcess relies on SIGCHLD being delivered to its signal
> > handler. Unlike VLC, we don't try to mask it to one thread only. Instead, the
> > signal handler function is written so that it can be run on any thread, and it
> > signals an auxiliary thread (the QProcessManager thread) to do the non-async-
> > safe operations.
> 
> Which is a much much saner thing to do than what the VLC folks chose
> to do in their code.
> 
> > So for QProcess to work, it requires that people don't disrupt the above.
> > 
> > Setting signal masks is fine as long as you don't block it -- the signal
> > handler will still be invoked in one thread. If you override with a different
> > handler, then at least ensure Qt's is run.
> 
> They simply seem to block it and not be concerned with it at all. At
> least that is what it looks like to me, not only from the code but
> also the comments written in the code.
> 
> > If the code you pasted from VLC is run after QCoreApplication's constructor,
> > the signal(SIGCHLD, SIG_DFL); call removes QProcessManager support.
> 
> The above snippet of code is something I clipped from
> http://git.videolan.org/?p=vlc.git;a=blob;f=bin/vlc.c;h=70eeed0d2299e2f3cf0c2f6819120f02d0703152;hb=HEAD
>  
> One cannot easily tell where QCoreApplication's ctor is getting
> called, but I suspect it is sometime after the call to libvlc_new(...)
> at line #201 and hence after all these signal blocking has occurred ??
> Since I was really not sure and did not want to waste to much time
> mucking around in a rather large and complex code base, I actually
> created a small test case to simulate the condition. I can clean that
> up and post it here if you want.

Here is the sample program I created to simulate this issue.

There is one more thing that happens... In my sample program, the
version information is correctly read once the timeout is reached.
However, that does not happen when using VLC, which means they are
doing even other more crazier things...


["test_qprocess.cpp" (text/x-c++src)]

#include <KDE/KStandardDirs>
#include <kdeversion.h>

#include <QtCore/QRegExp>
#include <QtCore/QDebug>
#include <QtCore/QProcess>
#include <QtCore/QReadWriteLock>
#include <QtCore/QRunnable>
#include <QtCore/QThreadPool>
#include <QtCore/QCoreApplication>

#include <cstdio>
#include <signal.h>

class ProcessTester : public QRunnable
{
public:
  ProcessTester(int instanceNum)
      : m_instanceNum(instanceNum)
  {
  }

  void run()
  {
    qDebug() << "Thread" << m_instanceNum << ": Got mimeinfo database version #:" << \
sharedMimeInfoVersion();  }

  int sharedMimeInfoVersion()
  {
    // NOTE: The use of a mutex here is simply to emulate what \
KMimeTypeRepository::sharedMimeInfoVersion() does...  m_mutex.lockForWrite();
    const QString umd = \
KStandardDirs::findExe(QString::fromLatin1("update-mime-database"));  if \
(umd.isEmpty()) {  qWarning() << "update-mime-database not found!";
        m_sharedMimeInfoVersion = -1;
    } else {
        QProcess smi;
        smi.start(umd, QStringList() << QString::fromLatin1("-v"));
        qDebug() << "Thread" << m_instanceNum << ": process started successfully ?" \
                << smi.waitForStarted();
        qDebug() << "Thread" << m_instanceNum << ": process finished successfully ?" \
                << smi.waitForFinished();
        const QString out = QString::fromLocal8Bit(smi.readAllStandardError());
        QRegExp versionRe(QString::fromLatin1("update-mime-database \
\\(shared-mime-info\\) (\\d+)\\.(\\d+)(\\.(\\d+))?"));  if (versionRe.indexIn(out) > \
                -1) {
            m_sharedMimeInfoVersion = KDE_MAKE_VERSION(versionRe.cap(1).toInt(), \
versionRe.cap(2).toInt(), versionRe.cap(4).toInt());  } else {
            qWarning() << "Unexpected version scheme from update-mime-database -v: \
got" << out;  m_sharedMimeInfoVersion = -1;
        }
    }
    m_mutex.unlock();
    return m_sharedMimeInfoVersion;

  }

private:
    QReadWriteLock m_mutex;
    int m_sharedMimeInfoVersion;
    int m_instanceNum;
};

static void blockSysSignals()
{
    /* The so-called POSIX-compliant MacOS X reportedly processes SIGPIPE even
     * if it is blocked in all thread. Also some libraries want SIGPIPE blocked
     * as they have no clue about signal masks.
     * Note: this is NOT an excuse for not protecting against SIGPIPE. If
     * LibVLC runs outside of VLC, we cannot rely on this code snippet. */
    signal (SIGPIPE, SIG_IGN);
    /* Restore default for SIGCHLD in case parent ignores it. */
    signal (SIGCHLD, SIG_DFL);

    sigset_t set;
    sigemptyset (&set);

    /* Signals that cause a no-op:
     * - SIGPIPE might happen with sockets and would crash VLC. It MUST be
     *   blocked by any LibVLC-dependent application, not just VLC.
     * - SIGCHLD comes after exec*() (such as httpd CGI support) and must
     *   be dequeued to cleanup zombie processes.
     */
    sigaddset (&set, SIGPIPE);
    sigaddset (&set, SIGCHLD);

    /* Block all these signals */
    pthread_sigmask (SIG_BLOCK, &set, NULL);
}

int main (int argc, char** argv)
{
    QCoreApplication app (argc, argv);
    if (argc > 1 && app.arguments().at(1) == QLatin1String("--block"))
        blockSysSignals();

    QThreadPool* pool = QThreadPool::globalInstance();
    const int maxThreadCount = QThread::idealThreadCount();
    for (int i = 0; i < maxThreadCount; ++i) {
        ProcessTester* testRunnable = new ProcessTester(i);
        pool->start(testRunnable);
    }

    pool->waitForDone();
}

// g++ -lkdecore -o test_qprocess test_qprocess.cpp



[prev in list] [next in list] [prev in thread] [next in thread]