QWindowsPipeReader: determine pipe state before signaling

The 'pipeBroken' flag must be updated before emitting the readyRead()
signal to avoid deadlock of waitForReadyRead() inside slot connected
to readyRead().

Change-Id: Ie393fdd594c6691da6609ea18307589b7157c624
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

src/corelib/io/qwindowspipereader.cpp

@@ -408,6 +408,14 @@ bool QWindowsPipeReader::consumePendingAndEmit(bool allowWinActPosting)
 
     mutex.unlock();
 
+    // Trigger 'pipeBroken' only once. This flag must be updated before
+    // emitting the readyRead() signal. Otherwise, the read sequence will
+    // be considered not finished, and we may hang if a slot connected
+    // to readyRead() calls waitForReadyRead().
+    const bool emitPipeClosed = (dwError != ERROR_SUCCESS && !pipeBroken);
+    if (emitPipeClosed)
+        pipeBroken = true;
+
     // Disable any further processing, if the pipe was stopped.
     // We are not allowed to emit signals in either 'Stopped'
     // or 'Draining' state.
@@ -418,10 +426,7 @@ bool QWindowsPipeReader::consumePendingAndEmit(bool allowWinActPosting)
         QScopedValueRollback<bool> guard(inReadyRead, true);
         emit readyRead();
     }
-
-    // Trigger 'pipeBroken' only once.
-    if (dwError != ERROR_SUCCESS && !pipeBroken) {
-        pipeBroken = true;
+    if (emitPipeClosed) {
         if (dwError != ERROR_BROKEN_PIPE && dwError != ERROR_PIPE_NOT_CONNECTED)
             emit winError(dwError, QLatin1String("QWindowsPipeReader::consumePendingAndEmit"));
         emit pipeClosed();

tests/auto/network/socket/qlocalsocket/tst_qlocalsocket.cpp

@@ -111,6 +111,7 @@ private slots:
     void longPath();
     void waitForDisconnect();
     void waitForDisconnectByServer();
+    void waitForReadyReadOnDisconnected();
 
     void removeServer();
 
@@ -1151,6 +1152,40 @@ void tst_QLocalSocket::waitForDisconnectByServer()
     QCOMPARE(spy.count(), 1);
 }
 
+void tst_QLocalSocket::waitForReadyReadOnDisconnected()
+{
+    QString name = "tst_localsocket";
+    LocalServer server;
+    QVERIFY(server.listen(name));
+    LocalSocket socket;
+    connect(&socket, &QLocalSocket::readyRead, [&socket]() {
+        QVERIFY(socket.getChar(nullptr));
+        // The next call should not block because the socket was closed
+        // by the peer.
+        QVERIFY(!socket.waitForReadyRead(3000));
+    });
+
+    socket.connectToServer(name);
+    QVERIFY(socket.waitForConnected(3000));
+    QVERIFY(server.waitForNewConnection(3000));
+    QLocalSocket *serverSocket = server.nextPendingConnection();
+    QVERIFY(serverSocket);
+    QVERIFY(serverSocket->putChar(0));
+    QVERIFY(serverSocket->waitForBytesWritten(3000));
+    serverSocket->close();
+
+#ifdef Q_OS_WIN
+    // Ensure that the asynchronously delivered close notification is
+    // already queued up before we consume the data.
+    QTest::qSleep(250);
+#endif
+
+    QElapsedTimer timer;
+    timer.start();
+    QVERIFY(socket.waitForReadyRead(5000));
+    QVERIFY(timer.elapsed() < 2000);
+}
+
 void tst_QLocalSocket::removeServer()
 {
     // this is a hostile takeover, but recovering from a crash results in the same