[Interest] Is QProcess really reentrant?

[Thiago Macieira]

On terça-feira, 23 de julho de 2013 21:58:03, Bill Crocker wrote:
> (gdb) where
> #0  0x08bae58e in QObjectPrivate::QObjectPrivate(int) ()
> #1  0x08bae890 in QObject::QObject(QObject*) ()
> #2  0x08bb49ff in QSocketNotifier::QSocketNotifier(int,
> QSocketNotifier::Type, QObject*) ()
> #3  0x08b7d720 in QProcessPrivate::createChannel(QProcessPrivate::Channel&)
> () #4  0x08b7db4a in QProcessPrivate::startProcess() ()
> #5  0x08b3d922 in QProcess::start(QString const&, QStringList const&,
> QFlags<QIODevice::OpenModeFlag>) ()
> #6  0x081058ce in GetDirectorySpace(QString, QString, int*, int*) ()
> #7  0x08297893 in GetDirectorySpaceThread::run() ()
> #8  0x08abd9af in QThreadPrivate::start(void*) ()
> #9  0x00a2c832 in start_thread () from /lib/libpthread.so.0
> #10 0x009970ae in clone () from /lib/libc.so.6

Debug symbols are missing, which unfortunately means we don't get line 
numbers... we can try, though.

Looking at the log for QProcess since 4.7.1, there *is* one thread-safety fix, 
which was released in Qt 4.8.5, but it wouldn't cause a crash. In the worst 
case, it would cause the program to terminate with SIGPIPE, but that's not the 
problem you're having -- we'd see one thread crashed on write(2) if it were 
the case.

No, your crashed thread is stopped in QObjectPrivate's constructor. There's 
nothing in the logs indicating a thread-safety fix and there are no known 
outstanding issues there. 

My first reaction was to blame a misuse of object-thread association, but that 
can't explain the issue. For that to be the case, we'd have to have two 
objects accessing the same internals. But that's not possible here because 
we're inside the QObject constructor -- in fact, we're in the QObject 
initialisation list. The code that tries to associate the thread to the object 
happens slightly after that, in the QObject constructor body.

Here's the interesting thing: QObjectPrivate's constructor does nothing that 
could cause a crash. Knowing how GCC works with inlining, if it is showing the 
frame for QObjectPrivate::QObjectPrivate, it really is in that function. 
Therefore, we must conclude one of:

1) the backtrace is faulty and we're misreading it

2) the crash really happened in QObjectPrivate due to its own fault

3) the crash happened in QObjectPrivate because it got passed a faulty this 
pointer.

4) the crash happened in QObjectPrivate, its this pointer is valid, but some 
other thread is smashing the memory

Option 1 is indeed a possibility, because you showed the backtrace for 10 
threads, but at no point did you tell us which thread had crashed. I'm 
assuming it's the one running code that isn't sleeping (QMutex::lock does a 
quick busy-loop trying to acquire the mutex).

Option 2 could be because of inlined code from a member's constructor. Since 
QObjectPrivate has one QString and one QList as members (besides primitives) 
as of 4.7.1, and we know that both of those constructors simply assign a 
constant to the d pointer, then do an atomic increment, Option 2 is extremely 
unlikely.

Option 3 is also a possibility, though. Under ordinary circumstances, I 
wouldn't say that, though. Look at the QObject constructor:
	QObject::QObject(QObject *parent)
	    : d_ptr(new QObjectPrivate)

For the this pointer to be invalid, we require operator new() to have returned 
an invalid pointer. In other words, for option 3 to be a possibility, the 
thread-safety bug needs to be in your system's malloc. I would not normally 
not go there, if it wasn't for the fact that threads 2, 3, 4, 6, and 7 are 
stopped *inside* malloc. They are inside malloc_atfork while thread 5 is doing 
fork. At the very least, that's suspicious.

Option 4 is always a possibility. None of the running threads are stopped at a 
place where they could be doing smashing, but that proves nothing. We don't 
know what they were doing before. And without pointer values, it's hard to say 
either way.

That's as far as I can go with the data available.

However, I have an extra comment, which is about the design of your code: why 
are you using threads to run processes? By their very nature, processes are 
asynchronous, so you're now doing double indirection of whatever needed to be 
done. I imagine that you have some legacy code in those GetSwap, GetActivity 
and GetDirectorySpace functions that tries to run QProcess in synchronous 
mode. So my suggestion is to refactor and remove the waitForFinished() calls. 
Simply start() the process, wait for the finished() signal and handle it there.

Hint: you can get a backtrace of all threads with the gdb command "thread 
apply all backtrace" (it can be shortened to "t a a bt")

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.qt-project.org/pipermail/interest/attachments/20130723/83ab04a3/attachment.sig>