From 5d68858ba78f485e19e2c36b15f86c8cc8435ff9 Mon Sep 17 00:00:00 2001
From: Alex Trotsenko <alex1973tr@gmail.com>
Date: Fri, 20 Aug 2021 19:15:26 +0300
Subject: [PATCH] QLocalSocket/Win: fix waitFor...() functions for write-only
 socket

There were several issues with the socket state checking when the pipe
reader is not running:

  - the number of object handles in the WaitForMultipleObjectsEx()
    call might have been zero;
  - a call to the waitForDisconnected(-1) might have hung;
  - we did not perform a loop iteration for the waitFor...(0) calls,
    so disconnect detection was unreliable.

These issues are related to the same code, so they don't seem to be
addressable separately.

Change-Id: I3bca872bb4191e6a7d38a693d81f7981af7fe145
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 src/network/socket/qlocalsocket_win.cpp       | 46 +++++++++++++++----
 .../socket/qlocalsocket/tst_qlocalsocket.cpp  |  7 +++
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/src/network/socket/qlocalsocket_win.cpp b/src/network/socket/qlocalsocket_win.cpp
index 229efe0226..0a39e84c07 100644
--- a/src/network/socket/qlocalsocket_win.cpp
+++ b/src/network/socket/qlocalsocket_win.cpp
@@ -48,24 +48,38 @@ struct QSocketPoller
 {
     QSocketPoller(const QLocalSocketPrivate &socket);
 
+    qint64 getRemainingTime(const QDeadlineTimer &deadline) const;
     bool poll(const QDeadlineTimer &deadline);
 
     enum { maxHandles = 2 };
     HANDLE handles[maxHandles];
     DWORD handleCount = 0;
     bool waitForClose = false;
+    bool writePending = false;
 };
 
 QSocketPoller::QSocketPoller(const QLocalSocketPrivate &socket)
 {
-    if (socket.pipeWriter)
+    if (socket.pipeWriter && socket.pipeWriter->bytesToWrite() != 0) {
         handles[handleCount++] = socket.pipeWriter->syncEvent();
+        writePending = true;
+    }
     if (socket.pipeReader->isReadOperationActive())
         handles[handleCount++] = socket.pipeReader->syncEvent();
     else
         waitForClose = true;
 }
 
+qint64 QSocketPoller::getRemainingTime(const QDeadlineTimer &deadline) const
+{
+    const qint64 sleepTime = 10;
+    qint64 remainingTime = deadline.remainingTime();
+    if (waitForClose && (remainingTime > sleepTime || remainingTime == -1))
+        return sleepTime;
+
+    return remainingTime;
+}
+
 /*!
     Waits until new data is available for reading or write operation
     completes. Returns \c true, if we need to check pipe workers;
@@ -77,9 +91,8 @@ QSocketPoller::QSocketPoller(const QLocalSocketPrivate &socket)
 */
 bool QSocketPoller::poll(const QDeadlineTimer &deadline)
 {
-    const qint64 sleepTime = 10;
-    QDeadlineTimer timer(waitForClose ? qMin(deadline.remainingTime(), sleepTime)
-                                      : deadline.remainingTime());
+    Q_ASSERT(handleCount != 0);
+    QDeadlineTimer timer(getRemainingTime(deadline));
     DWORD waitRet;
 
     do {
@@ -88,7 +101,7 @@ bool QSocketPoller::poll(const QDeadlineTimer &deadline)
     } while (waitRet == WAIT_IO_COMPLETION);
 
     if (waitRet == WAIT_TIMEOUT)
-        return !deadline.hasExpired();
+        return waitForClose || !deadline.hasExpired();
 
     return waitRet - WAIT_OBJECT_0 < handleCount;
 }
@@ -473,11 +486,23 @@ bool QLocalSocket::waitForDisconnected(int msecs)
     }
 
     QDeadlineTimer deadline(msecs);
+    bool wasChecked = false;
     while (!d->pipeReader->isPipeClosed()) {
-        QSocketPoller poller(*d);
-        if (!poller.poll(deadline))
+        if (wasChecked && deadline.hasExpired())
             return false;
 
+        QSocketPoller poller(*d);
+        // The first parameter of the WaitForMultipleObjectsEx() call cannot
+        // be zero. So we have to call SleepEx() here.
+        if (!poller.writePending && poller.waitForClose) {
+            // Prevent waiting on the first pass, if both the pipe reader
+            // and the pipe writer are inactive.
+            if (wasChecked)
+                SleepEx(poller.getRemainingTime(deadline), TRUE);
+        } else if (!poller.poll(deadline)) {
+            return false;
+        }
+
         if (d->pipeWriter)
             d->pipeWriter->checkForWrite();
 
@@ -487,6 +512,7 @@ bool QLocalSocket::waitForDisconnected(int msecs)
             d->pipeReader->checkPipeState();
 
         d->pipeReader->checkForReadyRead();
+        wasChecked = true;
     }
     d->_q_pipeClosed();
     return true;
@@ -529,12 +555,13 @@ bool QLocalSocket::waitForBytesWritten(int msecs)
         return false;
 
     QDeadlineTimer deadline(msecs);
+    bool wasChecked = false;
     while (!d->pipeReader->isPipeClosed()) {
-        if (bytesToWrite() == 0)
+        if (wasChecked && deadline.hasExpired())
             return false;
 
         QSocketPoller poller(*d);
-        if (!poller.poll(deadline))
+        if (!poller.writePending || !poller.poll(deadline))
             return false;
 
         Q_ASSERT(d->pipeWriter);
@@ -545,6 +572,7 @@ bool QLocalSocket::waitForBytesWritten(int msecs)
             d->pipeReader->checkPipeState();
 
         d->pipeReader->checkForReadyRead();
+        wasChecked = true;
     }
     d->_q_pipeClosed();
     return false;
diff --git a/tests/auto/network/socket/qlocalsocket/tst_qlocalsocket.cpp b/tests/auto/network/socket/qlocalsocket/tst_qlocalsocket.cpp
index 97214a5914..f537bd9157 100644
--- a/tests/auto/network/socket/qlocalsocket/tst_qlocalsocket.cpp
+++ b/tests/auto/network/socket/qlocalsocket/tst_qlocalsocket.cpp
@@ -1479,6 +1479,13 @@ void tst_QLocalSocket::writeOnlySocket()
 
     QCOMPARE(client.bytesAvailable(), qint64(0));
     QCOMPARE(client.state(), QLocalSocket::ConnectedState);
+
+    serverSocket->abort();
+    // On Windows, we need to test that the socket state is periodically
+    // checked in a loop, even if no timeout value is specified (i.e.
+    // waitForDisconnected(-1) does not fail immediately).
+    QVERIFY(client.waitForDisconnected(-1));
+    QCOMPARE(client.state(), QLocalSocket::UnconnectedState);
 }
 
 void tst_QLocalSocket::writeToClientAndDisconnect_data()