Changeset 188

Timestamp:

Nov 21, 2008, 4:36:19 AM (17 years ago)

Author:

dmik

Message:

Kernel: QPrcess: Another attempt to fix unexpected pipe handle reusal (ticket:51).

File:

• trunk/src/kernel/qprocess_pm.cpp (modified) (10 diffs)

trunk/src/kernel/qprocess_pm.cpp

@@ -143 +143 @@
     QProcessPrivate( QProcess *proc )
     {
-        stdinBufRead = 0;
+        stdinBufRead = 0;
         pipeStdin = HP_NULL;
         pid = PID_NULL;
-        exitValuesCalculated = FALSE;
-
-        lookup = new QTimer( proc );
-        qApp->connect( lookup, SIGNAL(timeout()), proc, SLOT(timeout()) );
+        exitValuesCalculated = FALSE;
+
+        lookup = new QTimer( proc );
+        qApp->connect( lookup, SIGNAL(timeout()), proc, SLOT(timeout()) );
     }
 
     ~QProcessPrivate()
     {
-        reset();
+        reset();
     }
 
     void reset()
     {
-        while ( !stdinBuf.isEmpty() ) {
-            delete stdinBuf.dequeue();
-        }
-        stdinBufRead = 0;
-        closeHandles();
+        while ( !stdinBuf.isEmpty() ) {
+            delete stdinBuf.dequeue();
+        }
+        stdinBufRead = 0;
+        closeHandles();
         stdout.buf.clear();
         stderr.buf.clear();
-        pid = PID_NULL;
-        exitValuesCalculated = FALSE;
+        pid = PID_NULL;
+        exitValuesCalculated = FALSE;
     }
 
@@ -237 +237 @@
     bool addProcess( QProcess *proc );
     void removeProcess( QProcessPrivate *d,
-                        QProcessPrivate::Pipe *pipe = NULL );
+                        QProcessPrivate::Pipe *pipe = NULL,
+                        bool inMsgHandler = false);
 
     void monitor();
@@ -405 +406 @@
 
 void QProcessMonitor::removeProcess( QProcessPrivate *d,
-                                     QProcessPrivate::Pipe *pipe /* = NULL */)
+                                     QProcessPrivate::Pipe *pipe /* = NULL */,
+                                     bool inMsgHandler /* = false */)
 {
 #if defined(QT_QPROCESS_DEBUG)
@@ -424 +426 @@
         return;
     }
-    
+
+    if ( !inMsgHandler ) {
+        // process all messages in the message queue related to QProcessMonitor.
+        // A failure to do so may lead to a situation when another QProcess
+        // started before these messages are processed but after our ends of
+        // pipes are closed (for example, by QProcessPrivate::reset()) gets the
+        // same pipe handles from the system. Since pipe handles are also used
+        // as keys, the delayed messages will then refer to the new QProcess
+        // object they were not intended for. The fix is to remove "our"
+        // messages from the queue now, before closing pipes (which will usually
+        // be done right after this method).
+    
+        QMSG qmsg;
+        while (WinPeekMsg (0, &qmsg, hwnd(), WM_U_PIPE_RDATA, WM_U_PIPE_CLOSE,
+                           PM_REMOVE)) {
+            qDebug( "msg=%08x", qmsg.msg );
+            if ( qmsg.msg == WM_U_PIPE_CLOSE ) {
+                QProcess *proc = (QProcess *) PVOIDFROMMP( qmsg.mp1 );
+                USHORT key = SHORT1FROMMP( qmsg.mp2 );
+                if ( proc == pipeKeys.find( key ) && proc->d == d ) {
+                    // skip the close message for ourselves as we will do all
+                    // the necessary stuff
+                    continue;
+                }
+            }
+            WinDispatchMsg( 0, &qmsg );
+        }
+    }
+
     if ( pipe ) {
         removePipe( pipe );
@@ -559 +589 @@
             if ( proc == pipeKeys.find( key ) ) {
 #if defined(QT_QPROCESS_DEBUG)
-            qDebug( "QProcessMonitor::WM_U_PIPE_RDATA: proc=%p (%s/%s) d=%p "
-                    "key=%04hX",
-                    proc, proc->name(), proc->className(), proc->d, key );
+                qDebug( "QProcessMonitor::WM_U_PIPE_RDATA: proc=%p (%s/%s) d=%p "
+                        "key=%04hX",
+                        proc, proc->name(), proc->className(), proc->d, key );
 #endif
                 QProcessPrivate *d = proc->d;
@@ -596 +626 @@
                     Q_ASSERT( pipe->closed );
                     // remove the single pipe from watching
-                    removeProcess( d, pipe ); 
+                    removeProcess( d, pipe, TRUE /* inMsgHandler */ ); 
                     // close the pipe since no more necessary
                     // (pipe is not watched anymore, no need to lock access)
@@ -686 +716 @@
         // the client's end of pipe has been closed, so close our end as well
         if ( processMonitor && pipe->key != KEY_NULL ) {
-            // WM_U_PIPE_CLOSE has been posted but not yet processed: do
-            // nothing in this case and let it do the close job. This is vital
-            // since if we close the pipe now then it's possible that another
-            // QProcess instance created in the meantime will get a pipe with
-            // the same handle from the system (and therefore with the same key)
-            // so our pending WM_U_PIPE_CLOSE message will attempt to close an
-            // invalid (foreign) pipe
-        }
-        else 
+            // WM_U_PIPE_CLOSE has been posted but not yet processed
+            // (the following call make sure it will be processed)
+            processMonitor->removeProcess( this, pipe );
+        }
+        else
             closePipe( pipe );
     }
@@ -770 +796 @@
 
     if ( _arguments.isEmpty() )
-        return FALSE;
+        return FALSE;
 
 #if defined(QT_QPROCESS_USE_DOSEXECPGM)
@@ -1479 +1505 @@
     QProcess *that = (QProcess *) this;
 
+    /// @todo the below comment is not relevant any more since removeProcess()
+    //  will dispatch all related messages including WM_U_PIPE_RDATA. Therefore,
+    //  this code should not be needed anymore.
+#if 0
     // There might be data to read, but WM_U_PIPE_RDATA messages won't be
     // converted to signals after removeProcess() is called below. Therefore,
@@ -1487 +1517 @@
     if ( d->readPipe( &d->stderr ) )
         emit that->readyReadStderr();
+#endif
     
     // compute the exit values