Do not wait in QWindowsPipe{Reader|Writer}::stop()

A deadlock can occur if the user does

    QLocalSocket *ls = new QLocalSocket;
    ls->moveToThread(t);
    ...
    delete ls;

Then QLocalSocket calls QWindowsPipeReader::stop() in a different thread
than the I/O operation is running in. The waitForNotified(-1) call would
then wait indefinitely until the I/O thread is in alertable wait state
again. Especially on application shut down this might never be the case,
and the application would deadlock.

Solve this by detaching the Overlapped object from the
QWindowsPipe{Reader|Writer} in stop() and delete it in the callback.

Task-number: QTBUG-61643
Change-Id: Ie262d75c5fd92ac7cf7dfcdbf1519050be9fd3c4
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@qt.io>

4 files changed, 30 insertions, 14 deletions

diff --git a/src/corelib/io/qwindowspipereader.cpp b/src/corelib/io/qwindowspipereader.cpp
index 4065b19446..b93fed5ba8 100644
--- a/src/corelib/io/qwindowspipereader.cpp
+++ b/src/corelib/io/qwindowspipereader.cpp
@@ -57,7 +57,7 @@ void QWindowsPipeReader::Overlapped::clear()
 QWindowsPipeReader::QWindowsPipeReader(QObject *parent)
     : QObject(parent),
       handle(INVALID_HANDLE_VALUE),
-      overlapped(this),
+      overlapped(nullptr),
       readBufferMaxSize(0),
       actualReadBufferSize(0),
       stopped(true),
@@ -74,6 +74,7 @@ QWindowsPipeReader::QWindowsPipeReader(QObject *parent)
 QWindowsPipeReader::~QWindowsPipeReader()
 {
     stop();
+    delete overlapped;
 }
 
 /*!
@@ -95,14 +96,16 @@ void QWindowsPipeReader::stop()
 {
     stopped = true;
     if (readSequenceStarted) {
-        if (!CancelIoEx(handle, &overlapped)) {
+        overlapped->pipeReader = nullptr;
+        if (!CancelIoEx(handle, overlapped)) {
             const DWORD dwError = GetLastError();
             if (dwError != ERROR_NOT_FOUND) {
                 qErrnoWarning(dwError, "QWindowsPipeReader: CancelIoEx on handle %p failed.",
                               handle);
             }
         }
-        waitForNotification(-1);
+        overlapped = nullptr;       // The object will be deleted in the I/O callback.
+        readSequenceStarted = false;
     }
 }
 
@@ -232,8 +235,10 @@ void QWindowsPipeReader::startAsyncRead()
 
     stopped = false;
     readSequenceStarted = true;
-    overlapped.clear();
-    if (!ReadFileEx(handle, ptr, bytesToRead, &overlapped, &readFileCompleted)) {
+    if (!overlapped)
+        overlapped = new Overlapped(this);
+    overlapped->clear();
+    if (!ReadFileEx(handle, ptr, bytesToRead, overlapped, &readFileCompleted)) {
         readSequenceStarted = false;
 
         const DWORD dwError = GetLastError();
@@ -260,7 +265,10 @@ void QWindowsPipeReader::readFileCompleted(DWORD errorCode, DWORD numberOfBytesT
                                            OVERLAPPED *overlappedBase)
 {
     Overlapped *overlapped = static_cast<Overlapped *>(overlappedBase);
-    overlapped->pipeReader->notified(errorCode, numberOfBytesTransfered);
+    if (overlapped->pipeReader)
+        overlapped->pipeReader->notified(errorCode, numberOfBytesTransfered);
+    else
+        delete overlapped;
 }
 
 /*!
diff --git a/src/corelib/io/qwindowspipereader_p.h b/src/corelib/io/qwindowspipereader_p.h
index 74ff5250ac..e52aa4c33d 100644
--- a/src/corelib/io/qwindowspipereader_p.h
+++ b/src/corelib/io/qwindowspipereader_p.h
@@ -105,7 +105,7 @@ private:
     };
 
     HANDLE handle;
-    Overlapped overlapped;
+    Overlapped *overlapped;
     qint64 readBufferMaxSize;
     QRingBuffer readBuffer;
     qint64 actualReadBufferSize;
diff --git a/src/corelib/io/qwindowspipewriter.cpp b/src/corelib/io/qwindowspipewriter.cpp
index d6ee4741e3..7eb6dd0389 100644
--- a/src/corelib/io/qwindowspipewriter.cpp
+++ b/src/corelib/io/qwindowspipewriter.cpp
@@ -56,7 +56,7 @@ void QWindowsPipeWriter::Overlapped::clear()
 QWindowsPipeWriter::QWindowsPipeWriter(HANDLE pipeWriteEnd, QObject *parent)
     : QObject(parent),
       handle(pipeWriteEnd),
-      overlapped(this),
+      overlapped(nullptr),
       numberOfBytesToWrite(0),
       pendingBytesWrittenValue(0),
       stopped(true),
@@ -72,6 +72,7 @@ QWindowsPipeWriter::QWindowsPipeWriter(HANDLE pipeWriteEnd, QObject *parent)
 QWindowsPipeWriter::~QWindowsPipeWriter()
 {
     stop();
+    delete overlapped;
 }
 
 bool QWindowsPipeWriter::waitForWrite(int msecs)
@@ -122,7 +123,10 @@ void QWindowsPipeWriter::writeFileCompleted(DWORD errorCode, DWORD numberOfBytes
                                             OVERLAPPED *overlappedBase)
 {
     Overlapped *overlapped = static_cast<Overlapped *>(overlappedBase);
-    overlapped->pipeWriter->notified(errorCode, numberOfBytesTransfered);
+    if (overlapped->pipeWriter)
+        overlapped->pipeWriter->notified(errorCode, numberOfBytesTransfered);
+    else
+        delete overlapped;
 }
 
 /*!
@@ -185,13 +189,15 @@ bool QWindowsPipeWriter::write(const QByteArray &ba)
     if (writeSequenceStarted)
         return false;
 
-    overlapped.clear();
+    if (!overlapped)
+        overlapped = new Overlapped(this);
+    overlapped->clear();
     buffer = ba;
     numberOfBytesToWrite = buffer.size();
     stopped = false;
     writeSequenceStarted = true;
     if (!WriteFileEx(handle, buffer.constData(), numberOfBytesToWrite,
-                     &overlapped, &writeFileCompleted)) {
+                     overlapped, &writeFileCompleted)) {
         writeSequenceStarted = false;
         numberOfBytesToWrite = 0;
         buffer.clear();
@@ -208,14 +214,16 @@ void QWindowsPipeWriter::stop()
     bytesWrittenPending = false;
     pendingBytesWrittenValue = 0;
     if (writeSequenceStarted) {
-        if (!CancelIoEx(handle, &overlapped)) {
+        overlapped->pipeWriter = nullptr;
+        if (!CancelIoEx(handle, overlapped)) {
             const DWORD dwError = GetLastError();
             if (dwError != ERROR_NOT_FOUND) {
                 qErrnoWarning(dwError, "QWindowsPipeWriter: CancelIoEx on handle %p failed.",
                               handle);
             }
         }
-        waitForNotification(-1);
+        overlapped = nullptr;       // The object will be deleted in the I/O callback.
+        writeSequenceStarted = false;
     }
 }
 
diff --git a/src/corelib/io/qwindowspipewriter_p.h b/src/corelib/io/qwindowspipewriter_p.h
index c43e986f34..c8252e2a4b 100644
--- a/src/corelib/io/qwindowspipewriter_p.h
+++ b/src/corelib/io/qwindowspipewriter_p.h
@@ -143,7 +143,7 @@ private:
     };
 
     HANDLE handle;
-    Overlapped overlapped;
+    Overlapped *overlapped;
     QByteArray buffer;
     qint64 numberOfBytesToWrite;
     qint64 pendingBytesWrittenValue;