[Qt-interest] QProcess could close the reading pipe, thus killing the child process with SIGPIPE

[Thiago Macieira]

Thiago Macieira wrote:
>The code you posted seemed right at first glance, except for the
>waitForReadyRead calls (and the use of a thread in the first place). The
>use of a thread points to the only other possibility here: there's a
>threading mismatch and the QIODevice object is actually being touched
>from outside its thread. I'm going to take a quick look at your code.

In Imap/Parser/Parser.cpp:
Parser::Parser( QObject* parent, Imap::Mailbox::SocketFactoryPtr factory 
):
        QObject(parent), _factory(factory), _lastTagUsed(0), 
_workerThread( this ),

This sets WorkerThread::_parser to "this". The Parser object is a QObject 
whose thread affinity is the calling thread (not the worker thread).

In Parser.h:
        WorkerHelper( Parser* parser ): _parser(parser) {};

This does the same thing. In both cases, you're passing QObject pointers 
as parameters to QObject-derived class constructors, but in neither case 
it's set as the parent QObject. That is quite confusing, because when I 
read:
	_workerThread(this)
or	helper = new WorkerHelper( _parser );
I think it's setting the parent

Note: the semicolon after {} is wrong.

In any case, the WorkerHelper class is created inside the run() function, 
so its thread-affinity is the worker thread.

In Streams/SocketFactory.cpp, all create() functions create QIODevice 
derivatives without a parent, so they all also have affinity to the worker 
thread.

Since both sender (the QIODevice) and receiver are in the same thread, the 
delivering of the queued signal will interrupt any work done in the file 
descriptor. That also means Parser::processLine will be called and run 
outside its thread.

That means Parser::processLine and any functions it calls are safe.

What bothers me is the executeACommand() function, which accesses the 
socket. From your construction, it seems that that function is called only 
after the commandQueued signal is emitted, which is delivered to the 
helper object (which, as we established, has affinity to the worker thread). 
That means the signal is delivered via a queued connection. (btw, in 
executeIfPossible you unnecessarily unlocks the mutex).

As I said before, there's nothing that stands out to the eye. But there's 
a lot of things I wouldn't do, starting with the use of a thread. If you 
remove the reason you added the thread in the first place (the 
waitForBytesWritten and waitForReadyRead calls), you won't need a thread 
and your problem should disappear.

-- 
Thiago Macieira - thiago.macieira (AT) nokia.com
  Senior Product Manager - Nokia, Qt Software
      Sandakerveien 116, NO-0402 Oslo, Norway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
Url : http://lists.qt-project.org/pipermail/qt-interest-old/attachments/20090503/4f0529d4/attachment.bin