QLocalSocket/Win: fix closed state detection in waitFor...() functions

A delayed close should only be completed in the _q_bytesWritten() slot
as a confirmation of a successful write operation on the socket.
Otherwise, a failed write operation may cause the socket to be closed
unexpectedly within the waitFor...() function, which may result in a
malfunction.

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

src/corelib/io/qwindowspipereader.cpp

@@ -58,7 +58,7 @@ QWindowsPipeReader::QWindowsPipeReader(QObject *parent)
       lastError(ERROR_SUCCESS),
       state(Stopped),
       readSequenceStarted(false),
-      pipeBroken(false),
+      pipeBroken(true),
       readyReadPending(false),
       winEventActPosted(false)
 {
@@ -112,10 +112,6 @@ void QWindowsPipeReader::stop()
 void QWindowsPipeReader::drainAndStop()
 {
     cancelAsyncRead(Draining);
-
-    // Note that signals are not emitted in the call below, as the caller
-    // is expected to do that synchronously.
-    consumePending();
 }
 
 /*!
@@ -123,10 +119,11 @@ void QWindowsPipeReader::drainAndStop()
  */
 void QWindowsPipeReader::cancelAsyncRead(State newState)
 {
+    pipeBroken = true;
     if (state != Running)
         return;
 
-    QMutexLocker locker(&mutex);
+    mutex.lock();
     state = newState;
     if (readSequenceStarted) {
         // This can legitimately fail due to the GetOverlappedResult()
@@ -142,11 +139,17 @@ void QWindowsPipeReader::cancelAsyncRead(State newState)
 
         // Wait for callback to complete.
         do {
-            locker.unlock();
+            mutex.unlock();
             waitForNotification();
-            locker.relock();
+            mutex.lock();
         } while (readSequenceStarted);
     }
+    mutex.unlock();
+
+    // Because pipeBroken was set earlier, finish reading to keep the class
+    // state consistent. Note that signals are not emitted in the call
+    // below, as the caller is expected to do that synchronously.
+    consumePending();
 }
 
 /*!

src/network/socket/qlocalsocket.h

@@ -147,7 +147,6 @@ private:
     Q_PRIVATE_SLOT(d_func(), void _q_stateChanged(QAbstractSocket::SocketState))
     Q_PRIVATE_SLOT(d_func(), void _q_errorOccurred(QAbstractSocket::SocketError))
 #elif defined(Q_OS_WIN)
-    Q_PRIVATE_SLOT(d_func(), void _q_canWrite())
     Q_PRIVATE_SLOT(d_func(), void _q_pipeClosed())
     Q_PRIVATE_SLOT(d_func(), void _q_winError(ulong, const QString &))
 #else

src/network/socket/qlocalsocket_p.h

@@ -136,7 +136,7 @@ public:
     qint64 pipeWriterBytesToWrite() const;
     void _q_canRead();
     void _q_bytesWritten(qint64 bytes);
-    void _q_canWrite();
+    void writeToSocket();
     void _q_pipeClosed();
     void _q_winError(ulong windowsError, const QString &function);
     HANDLE handle;

src/network/socket/qlocalsocket_win.cpp

@@ -282,7 +282,7 @@ qint64 QLocalSocket::writeData(const char *data, qint64 len)
         QObjectPrivate::connect(d->pipeWriter, &QWindowsPipeWriter::bytesWritten,
                                 d, &QLocalSocketPrivate::_q_bytesWritten);
     }
-    d->_q_canWrite();
+    d->writeToSocket();
     return len;
 }
 
@@ -434,22 +434,23 @@ void QLocalSocketPrivate::_q_bytesWritten(qint64 bytes)
         QScopedValueRollback<bool> guard(emittedBytesWritten, true);
         emit q->bytesWritten(bytes);
     }
-    _q_canWrite();
-}
-
-void QLocalSocketPrivate::_q_canWrite()
-{
-    Q_Q(QLocalSocket);
     if (writeBuffer.isEmpty()) {
         if (state == QLocalSocket::ClosingState && pipeWriterBytesToWrite() == 0)
             q->close();
     } else {
-        Q_ASSERT(pipeWriter);
-        if (!pipeWriter->isWriteOperationActive())
-            pipeWriter->write(writeBuffer.read());
+        writeToSocket();
     }
 }
 
+void QLocalSocketPrivate::writeToSocket()
+{
+    Q_ASSERT(pipeWriter);
+    Q_ASSERT(!writeBuffer.isEmpty());
+
+    if (!pipeWriter->isWriteOperationActive())
+        pipeWriter->write(writeBuffer.read());
+}
+
 qintptr QLocalSocket::socketDescriptor() const
 {
     Q_D(const QLocalSocket);
@@ -488,7 +489,8 @@ bool QLocalSocket::waitForDisconnected(int msecs)
 
     QDeadlineTimer deadline(msecs);
     while (!d->pipeReader->isPipeClosed()) {
-        d->_q_canWrite();
+        if (!d->writeBuffer.isEmpty())
+            d->writeToSocket();
 
         QSocketPoller poller(*d);
         if (!poller.poll(deadline))
@@ -499,7 +501,7 @@ bool QLocalSocket::waitForDisconnected(int msecs)
 
         // When the read buffer is full, the read sequence is not running,
         // so we need to peek the pipe to detect disconnection.
-        if (poller.waitForClose)
+        if (poller.waitForClose && isValid())
             d->pipeReader->checkPipeState();
 
         d->pipeReader->checkForReadyRead();
@@ -523,7 +525,8 @@ bool QLocalSocket::waitForReadyRead(int msecs)
 
     QDeadlineTimer deadline(msecs);
     while (!d->pipeReader->isPipeClosed()) {
-        d->_q_canWrite();
+        if (!d->writeBuffer.isEmpty())
+            d->writeToSocket();
 
         QSocketPoller poller(*d);
         if (poller.waitForClose || !poller.poll(deadline))
@@ -548,9 +551,11 @@ bool QLocalSocket::waitForBytesWritten(int msecs)
 
     QDeadlineTimer deadline(msecs);
     while (!d->pipeReader->isPipeClosed()) {
-        if (bytesToWrite() == 0)
+        if (!d->writeBuffer.isEmpty()) {
+            d->writeToSocket();
+        } else if (d->pipeWriterBytesToWrite() == 0) {
             return false;
-        d->_q_canWrite();
+        }
 
         QSocketPoller poller(*d);
         if (!poller.poll(deadline))