Fix crash in QProcess::waitForFinished on Windows

Suppose the user connects QProcess::readyReadStandardOutput with a
slot that calls QCoreApplication::processEvents.
Assume the event loop did not handle events between QProcess::start
and QProcess::waitForFinished. The process writes to stdout and exits.
QProcessPrivate::waitForFinished calls drainOutputPipes which calls
QWindowsPipeWriter::waitForReadyRead. This in turn will trigger
_q_processDied via the readyRead signal and processEvents.
_q_processDied will delete the pid object and set pid to null.
After drainOutputPipes returns, _q_processDied is called again but it
must not be called if pid is already destroyed.

Prevent calling _q_processDied if pid is null.

Task-number: QTBUG-48697
Change-Id: Iee047938ee1529057a1a43d71f4e882750903c7e
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>

2 files changed, 37 insertions, 2 deletions

diff --git a/src/corelib/io/qprocess_win.cpp b/src/corelib/io/qprocess_win.cpp
index 80e6d5bb61..98ada82446 100644
--- a/src/corelib/io/qprocess_win.cpp
+++ b/src/corelib/io/qprocess_win.cpp
@@ -647,7 +647,8 @@ bool QProcessPrivate::waitForReadyRead(int msecs)
             return false;
         if (WaitForSingleObjectEx(pid->hProcess, 0, false) == WAIT_OBJECT_0) {
             bool readyReadEmitted = drainOutputPipes();
-            _q_processDied();
+            if (pid)
+                _q_processDied();
             return readyReadEmitted;
         }
 
@@ -752,7 +753,8 @@ bool QProcessPrivate::waitForFinished(int msecs)
 
         if (WaitForSingleObject(pid->hProcess, timer.nextSleepTime()) == WAIT_OBJECT_0) {
             drainOutputPipes();
-            _q_processDied();
+            if (pid)
+                _q_processDied();
             return true;
         }
 
diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
index 8b8c731987..46bf1a19e8 100644
--- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
+++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
@@ -145,6 +145,8 @@ private slots:
     void startStopStartStop();
     void startStopStartStopBuffers_data();
     void startStopStartStopBuffers();
+    void processEventsInAReadyReadSlot_data();
+    void processEventsInAReadyReadSlot();
 
     // keep these at the end, since they use lots of processes and sometimes
     // caused obscure failures to occur in tests that followed them (esp. on the Mac)
@@ -157,6 +159,7 @@ private slots:
 protected slots:
     void readFromProcess();
     void exitLoopSlot();
+    void processApplicationEvents();
 #ifndef Q_OS_WINCE
     void restartProcess();
     void waitForReadyReadInAReadyReadSlotSlot();
@@ -475,6 +478,11 @@ void tst_QProcess::exitLoopSlot()
     QTestEventLoop::instance().exitLoop();
 }
 
+void tst_QProcess::processApplicationEvents()
+{
+    QCoreApplication::processEvents();
+}
+
 #ifndef Q_OS_WINCE
 // Reading and writing to a process is not supported on Qt/CE
 void tst_QProcess::echoTest2()
@@ -2499,6 +2507,31 @@ void tst_QProcess::startStopStartStopBuffers()
     }
 }
 
+void tst_QProcess::processEventsInAReadyReadSlot_data()
+{
+    QTest::addColumn<bool>("callWaitForReadyRead");
+
+    QTest::newRow("no waitForReadyRead") << false;
+    QTest::newRow("waitForReadyRead") << true;
+}
+
+void tst_QProcess::processEventsInAReadyReadSlot()
+{
+    // Test whether processing events in a readyReadXXX slot crashes. (QTBUG-48697)
+    QFETCH(bool, callWaitForReadyRead);
+    QProcess process;
+    QObject::connect(&process, &QProcess::readyReadStandardOutput,
+                     this, &tst_QProcess::processApplicationEvents);
+    process.start("testProcessEcho/testProcessEcho");
+    QVERIFY(process.waitForStarted());
+    const QByteArray data(156, 'x');
+    process.write(data.constData(), data.size() + 1);
+    if (callWaitForReadyRead)
+        QVERIFY(process.waitForReadyRead());
+    if (process.state() == QProcess::Running)
+        QVERIFY(process.waitForFinished());
+}
+
 #endif //QT_NO_PROCESS
 
 QTEST_MAIN(tst_QProcess)