QSerialPort: Port to alertable I/O functions on Windows

This commit removes the QWinOverlappedIoNotifier in favor of alertable
I/O functions like {Read|Write}FileEx(). The reason is that the
QWinOverlappedIoNotifier is very complex to maintain.

To use the alertable functions in the serial port we need an
alertable analog of WaitCommEvent function. This function does not
exist in the Win32 API, but we implement it trough a set of the
system NtXXX and RtlXXX functions, which are resolved dynamically.

This patch was tested with auto-tests and the examples using the:

* com0com virtual serial port driver.
* eltima virtual serial port driver.
* pl2303 USB/serial converters.

Task-number: QTBUG-74961
Change-Id: Idc428173eee7a1066a4693de00aa38416c4ee86c
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
Reviewed-by: Jörg Bornemann <joerg.bornemann@qt.io>
 

1 files changed, 248 insertions, 116 deletions

diff --git a/src/serialport/qserialport_win.cpp b/src/serialport/qserialport_win.cpp
index 85dd8ee2..da1e7aae 100644
--- a/src/serialport/qserialport_win.cpp
+++ b/src/serialport/qserialport_win.cpp
@@ -40,45 +40,15 @@
 ****************************************************************************/
 
 #include "qserialport_p.h"
-#include "qwinoverlappedionotifier_p.h"
+#include "qtntdll_p.h"
 
 #include <QtCore/qcoreevent.h>
 #include <QtCore/qelapsedtimer.h>
-#include <QtCore/qvector.h>
+#include <QtCore/qmutex.h>
 #include <QtCore/qtimer.h>
+#include <QtCore/qvector.h>
 #include <algorithm>
 
-#ifndef CTL_CODE
-#  define CTL_CODE(DeviceType, Function, Method, Access) ( \
-    ((DeviceType) << 16) | ((Access) << 14) | ((Function) << 2) | (Method) \
-    )
-#endif
-
-#ifndef FILE_DEVICE_SERIAL_PORT
-#  define FILE_DEVICE_SERIAL_PORT  27
-#endif
-
-#ifndef METHOD_BUFFERED
-#  define METHOD_BUFFERED  0
-#endif
-
-#ifndef FILE_ANY_ACCESS
-#  define FILE_ANY_ACCESS  0x00000000
-#endif
-
-#ifndef IOCTL_SERIAL_GET_DTRRTS
-#  define IOCTL_SERIAL_GET_DTRRTS \
-    CTL_CODE(FILE_DEVICE_SERIAL_PORT, 30, METHOD_BUFFERED, FILE_ANY_ACCESS)
-#endif
-
-#ifndef SERIAL_DTR_STATE
-#  define SERIAL_DTR_STATE  0x00000001
-#endif
-
-#ifndef SERIAL_RTS_STATE
-#  define SERIAL_RTS_STATE  0x00000002
-#endif
-
 QT_BEGIN_NAMESPACE
 
 static inline void qt_set_common_props(DCB *dcb)
@@ -173,8 +143,117 @@ static inline void qt_set_flowcontrol(DCB *dcb, QSerialPort::FlowControl flowcon
     }
 }
 
+// Translate NT-callbacks to Win32 callbacks.
+static VOID WINAPI qt_apc_routine(
+        PVOID context,
+        PIO_STATUS_BLOCK ioStatusBlock,
+        DWORD reserved)
+{
+    Q_UNUSED(reserved);
+
+    const DWORD errorCode = ::RtlNtStatusToDosError(ioStatusBlock->Status);
+    const DWORD bytesTransfered = NT_SUCCESS(ioStatusBlock->Status)
+            ? DWORD(ioStatusBlock->Information) : 0;
+    const LPOVERLAPPED overlapped = CONTAINING_RECORD(ioStatusBlock,
+                                                      OVERLAPPED, Internal);
+
+    (reinterpret_cast<LPOVERLAPPED_COMPLETION_ROUTINE>(context))
+            (errorCode, bytesTransfered, overlapped);
+}
+
+// Alertable analog of DeviceIoControl function.
+static BOOL qt_device_io_control_ex(
+        HANDLE deviceHandle,
+        DWORD ioControlCode,
+        LPVOID inputBuffer,
+        DWORD inputBufferSize,
+        LPVOID outputBuffer,
+        DWORD outputBufferSize,
+        LPOVERLAPPED overlapped,
+        LPOVERLAPPED_COMPLETION_ROUTINE completionRoutine)
+{
+    const auto ioStatusBlock = reinterpret_cast<PIO_STATUS_BLOCK>(
+                &overlapped->Internal);
+    ioStatusBlock->Status = STATUS_PENDING;
+
+    const NTSTATUS status = ::NtDeviceIoControlFile(
+                deviceHandle,
+                nullptr,
+                qt_apc_routine,
+                reinterpret_cast<PVOID>(completionRoutine),
+                ioStatusBlock,
+                ioControlCode,
+                inputBuffer,
+                inputBufferSize,
+                outputBuffer,
+                outputBufferSize);
+
+    if (!NT_SUCCESS(status)) {
+        ::SetLastError(::RtlNtStatusToDosError(status));
+        return false;
+    }
+
+    return true;
+}
+
+// Alertable analog of WaitCommEvent function.
+static BOOL qt_wait_comm_event_ex(
+        HANDLE deviceHandle,
+        LPDWORD eventsMask,
+        LPOVERLAPPED overlapped,
+        LPOVERLAPPED_COMPLETION_ROUTINE completionRoutine)
+{
+    return qt_device_io_control_ex(
+                deviceHandle,
+                IOCTL_SERIAL_WAIT_ON_MASK,
+                nullptr,
+                0,
+                eventsMask,
+                sizeof(DWORD),
+                overlapped,
+                completionRoutine);
+}
+
+struct RuntimeHelper
+{
+    QLibrary ntLibrary;
+    QBasicMutex mutex;
+};
+
+Q_GLOBAL_STATIC(RuntimeHelper, helper)
+
+class Overlapped final : public OVERLAPPED
+{
+    Q_DISABLE_COPY(Overlapped)
+public:
+    explicit Overlapped(QSerialPortPrivate *d);
+    void clear();
+
+    QSerialPortPrivate *dptr = nullptr;
+};
+
+Overlapped::Overlapped(QSerialPortPrivate *d)
+    : dptr(d)
+{
+}
+
+void Overlapped::clear()
+{
+    ::ZeroMemory(this, sizeof(OVERLAPPED));
+}
+
 bool QSerialPortPrivate::open(QIODevice::OpenMode mode)
 {
+    {
+        QMutexLocker locker(&helper()->mutex);
+        static bool symbolsResolved = resolveSymbols(&helper()->ntLibrary);
+        if (!symbolsResolved) {
+            setError(QSerialPortErrorInfo(QSerialPort::OpenError,
+                                          helper()->ntLibrary.errorString()));
+            return false;
+        }
+    }
+
     DWORD desiredAccess = 0;
 
     if (mode & QIODevice::ReadOnly)
@@ -199,17 +278,44 @@ bool QSerialPortPrivate::open(QIODevice::OpenMode mode)
 
 void QSerialPortPrivate::close()
 {
-    ::CancelIo(handle);
-
-    delete notifier;
-    notifier = nullptr;
-
     delete startAsyncWriteTimer;
     startAsyncWriteTimer = nullptr;
 
-    communicationStarted = false;
-    readStarted = false;
-    writeStarted = false;
+    if (communicationStarted) {
+        communicationCompletionOverlapped->dptr = nullptr;
+        ::CancelIoEx(handle, communicationCompletionOverlapped);
+        // The object will be deleted in the I/O callback.
+        communicationCompletionOverlapped = nullptr;
+        communicationStarted = false;
+    } else {
+        delete communicationCompletionOverlapped;
+        communicationCompletionOverlapped = nullptr;
+    }
+
+    if (readStarted) {
+        readCompletionOverlapped->dptr = nullptr;
+        ::CancelIoEx(handle, readCompletionOverlapped);
+        // The object will be deleted in the I/O callback.
+        readCompletionOverlapped = nullptr;
+        readStarted = false;
+    } else {
+        delete readCompletionOverlapped;
+        readCompletionOverlapped = nullptr;
+    };
+
+    if (writeStarted) {
+        writeCompletionOverlapped->dptr = nullptr;
+        ::CancelIoEx(handle, writeCompletionOverlapped);
+        // The object will be deleted in the I/O callback.
+        writeCompletionOverlapped = nullptr;
+        writeStarted = false;
+    } else {
+        delete writeCompletionOverlapped;
+        writeCompletionOverlapped = nullptr;
+    }
+
+    readBytesTransferred = 0;
+    writeBytesTransferred = 0;
     writeBuffer.clear();
 
     if (settingsRestoredOnClose) {
@@ -341,30 +447,25 @@ bool QSerialPortPrivate::waitForReadyRead(int msecs)
     if (!writeStarted && !_q_startAsyncWrite())
         return false;
 
-    const qint64 initialReadBufferSize = buffer.size();
-    qint64 currentReadBufferSize = initialReadBufferSize;
-
     QDeadlineTimer deadline(msecs);
 
     do {
-        const OVERLAPPED *overlapped = waitForNotified(deadline);
-        if (!overlapped)
-            return false;
-
-        if (overlapped == &readCompletionOverlapped) {
-            const qint64 readBytesForOneReadOperation = qint64(buffer.size()) - currentReadBufferSize;
-            if (readBytesForOneReadOperation == QSERIALPORT_BUFFERSIZE) {
-                currentReadBufferSize = buffer.size();
-            } else if (readBytesForOneReadOperation == 0) {
-                if (initialReadBufferSize != currentReadBufferSize)
-                    return true;
-            } else {
-                return true;
-            }
+        if (readBytesTransferred <= 0) {
+            const qint64 remaining = deadline.remainingTime();
+            const DWORD result = ::SleepEx(
+                        remaining == -1 ? INFINITE : DWORD(remaining),
+                        TRUE);
+            if (result != WAIT_IO_COMPLETION)
+                continue;
         }
 
+        if (readBytesTransferred > 0) {
+            readBytesTransferred = 0;
+            return true;
+        }
     } while (!deadline.hasExpired());
 
+    setError(getSystemError(WAIT_TIMEOUT));
     return false;
 }
 
@@ -378,15 +479,23 @@ bool QSerialPortPrivate::waitForBytesWritten(int msecs)
 
     QDeadlineTimer deadline(msecs);
 
-    for (;;) {
-        const OVERLAPPED *overlapped = waitForNotified(deadline);
-        if (!overlapped)
-            return false;
+    do {
+        if (writeBytesTransferred <= 0) {
+            const qint64 remaining = deadline.remainingTime();
+            const DWORD result = ::SleepEx(
+                        remaining == -1 ? INFINITE : DWORD(remaining),
+                        TRUE);
+            if (result != WAIT_IO_COMPLETION)
+                continue;
+        }
 
-         if (overlapped == &writeCompletionOverlapped)
+        if (writeBytesTransferred > 0) {
+            writeBytesTransferred = 0;
             return true;
-    }
+        }
+    } while (!deadline.hasExpired());
 
+    setError(getSystemError(WAIT_TIMEOUT));
     return false;
 }
 
@@ -467,6 +576,10 @@ bool QSerialPortPrivate::completeAsyncCommunication(qint64 bytesTransferred)
 
 bool QSerialPortPrivate::completeAsyncRead(qint64 bytesTransferred)
 {
+    // Store the number of transferred bytes which are
+    // required only in waitForReadyRead() method.
+    readBytesTransferred = bytesTransferred;
+
     if (bytesTransferred == qint64(-1)) {
         readStarted = false;
         return false;
@@ -494,6 +607,10 @@ bool QSerialPortPrivate::completeAsyncWrite(qint64 bytesTransferred)
 {
     Q_Q(QSerialPort);
 
+    // Store the number of transferred bytes which are
+    // required only in waitForBytesWritten() method.
+    writeBytesTransferred = bytesTransferred;
+
     if (writeStarted) {
         if (bytesTransferred == qint64(-1)) {
             writeChunkBuffer.clear();
@@ -514,8 +631,16 @@ bool QSerialPortPrivate::startAsyncCommunication()
     if (communicationStarted)
         return true;
 
-    ::ZeroMemory(&communicationOverlapped, sizeof(communicationOverlapped));
-    if (!::WaitCommEvent(handle, &triggeredEventMask, &communicationOverlapped)) {
+    if (!communicationCompletionOverlapped)
+        communicationCompletionOverlapped = new Overlapped(this);
+
+    communicationCompletionOverlapped->clear();
+    communicationStarted = true;
+    if (!::qt_wait_comm_event_ex(handle,
+                                 &triggeredEventMask,
+                                 communicationCompletionOverlapped,
+                                 ioCompletionRoutine)) {
+        communicationStarted = false;
         QSerialPortErrorInfo error = getSystemError();
         if (error.errorCode != QSerialPort::NoError) {
             if (error.errorCode == QSerialPort::PermissionError)
@@ -524,7 +649,6 @@ bool QSerialPortPrivate::startAsyncCommunication()
             return false;
         }
     }
-    communicationStarted = true;
     return true;
 }
 
@@ -546,23 +670,27 @@ bool QSerialPortPrivate::startAsyncRead()
 
     Q_ASSERT(int(bytesToRead) <= readChunkBuffer.size());
 
-    ::ZeroMemory(&readCompletionOverlapped, sizeof(readCompletionOverlapped));
-    if (::ReadFile(handle, readChunkBuffer.data(), bytesToRead, nullptr, &readCompletionOverlapped)) {
-        readStarted = true;
-        return true;
-    }
-
-    QSerialPortErrorInfo error = getSystemError();
-    if (error.errorCode != QSerialPort::NoError) {
-        if (error.errorCode == QSerialPort::PermissionError)
-            error.errorCode = QSerialPort::ResourceError;
-        if (error.errorCode != QSerialPort::ResourceError)
-            error.errorCode = QSerialPort::ReadError;
-        setError(error);
-        return false;
-    }
+    if (!readCompletionOverlapped)
+        readCompletionOverlapped = new Overlapped(this);
 
+    readCompletionOverlapped->clear();
     readStarted = true;
+    if (!::ReadFileEx(handle,
+                      readChunkBuffer.data(),
+                      bytesToRead,
+                      readCompletionOverlapped,
+                      ioCompletionRoutine)) {
+        readStarted = false;
+        QSerialPortErrorInfo error = getSystemError();
+        if (error.errorCode != QSerialPort::NoError) {
+            if (error.errorCode == QSerialPort::PermissionError)
+                error.errorCode = QSerialPort::ResourceError;
+            if (error.errorCode != QSerialPort::ResourceError)
+                error.errorCode = QSerialPort::ReadError;
+            setError(error);
+            return false;
+        }
+    }
     return true;
 }
 
@@ -572,10 +700,18 @@ bool QSerialPortPrivate::_q_startAsyncWrite()
         return true;
 
     writeChunkBuffer = writeBuffer.read();
-    ::ZeroMemory(&writeCompletionOverlapped, sizeof(writeCompletionOverlapped));
-    if (!::WriteFile(handle, writeChunkBuffer.constData(),
-                     writeChunkBuffer.size(), nullptr, &writeCompletionOverlapped)) {
 
+    if (!writeCompletionOverlapped)
+        writeCompletionOverlapped = new Overlapped(this);
+
+    writeCompletionOverlapped->clear();
+    writeStarted = true;
+    if (!::WriteFileEx(handle,
+                       writeChunkBuffer.constData(),
+                       writeChunkBuffer.size(),
+                       writeCompletionOverlapped,
+                       ioCompletionRoutine)) {
+        writeStarted = false;
         QSerialPortErrorInfo error = getSystemError();
         if (error.errorCode != QSerialPort::NoError) {
             if (error.errorCode != QSerialPort::ResourceError)
@@ -584,25 +720,29 @@ bool QSerialPortPrivate::_q_startAsyncWrite()
             return false;
         }
     }
-
-    writeStarted = true;
     return true;
 }
 
-void QSerialPortPrivate::_q_notified(DWORD numberOfBytes, DWORD errorCode, OVERLAPPED *overlapped)
+void QSerialPortPrivate::handleNotification(DWORD bytesTransferred, DWORD errorCode,
+                                            OVERLAPPED *overlapped)
 {
+    // This occurred e.g. after calling the CloseHandle() function,
+    // just skip handling at all.
+    if (handle == INVALID_HANDLE_VALUE)
+        return;
+
     const QSerialPortErrorInfo error = getSystemError(errorCode);
     if (error.errorCode != QSerialPort::NoError) {
         setError(error);
         return;
     }
 
-    if (overlapped == &communicationOverlapped)
-        completeAsyncCommunication(numberOfBytes);
-    else if (overlapped == &readCompletionOverlapped)
-        completeAsyncRead(numberOfBytes);
-    else if (overlapped == &writeCompletionOverlapped)
-        completeAsyncWrite(numberOfBytes);
+    if (overlapped == communicationCompletionOverlapped)
+        completeAsyncCommunication(bytesTransferred);
+    else if (overlapped == readCompletionOverlapped)
+        completeAsyncRead(bytesTransferred);
+    else if (overlapped == writeCompletionOverlapped)
+        completeAsyncWrite(bytesTransferred);
     else
         Q_ASSERT(!"Unknown OVERLAPPED activated");
 }
@@ -632,16 +772,6 @@ qint64 QSerialPortPrivate::writeData(const char *data, qint64 maxSize)
     return maxSize;
 }
 
-OVERLAPPED *QSerialPortPrivate::waitForNotified(QDeadlineTimer deadline)
-{
-    OVERLAPPED *overlapped = notifier->waitForAnyNotified(deadline);
-    if (!overlapped) {
-        setError(getSystemError(WAIT_TIMEOUT));
-        return nullptr;
-    }
-    return overlapped;
-}
-
 qint64 QSerialPortPrivate::queuedBytesCount(QSerialPort::Direction direction) const
 {
     COMSTAT comstat;
@@ -654,8 +784,6 @@ qint64 QSerialPortPrivate::queuedBytesCount(QSerialPort::Direction direction) co
 
 inline bool QSerialPortPrivate::initialize(QIODevice::OpenMode mode)
 {
-    Q_Q(QSerialPort);
-
     DCB dcb;
     if (!getDcb(&dcb))
         return false;
@@ -691,17 +819,8 @@ inline bool QSerialPortPrivate::initialize(QIODevice::OpenMode mode)
         return false;
     }
 
-    notifier = new QWinOverlappedIoNotifier(q);
-    QObjectPrivate::connect(notifier, &QWinOverlappedIoNotifier::notified,
-               this, &QSerialPortPrivate::_q_notified);
-    notifier->setHandle(handle);
-    notifier->setEnabled(true);
-
-    if ((eventMask & EV_RXCHAR) && !startAsyncCommunication()) {
-        delete notifier;
-        notifier = nullptr;
+    if ((eventMask & EV_RXCHAR) && !startAsyncCommunication())
         return false;
-    }
 
     return true;
 }
@@ -801,4 +920,17 @@ QSerialPort::Handle QSerialPort::handle() const
     return d->handle;
 }
 
+void QSerialPortPrivate::ioCompletionRoutine(
+        DWORD errorCode, DWORD bytesTransfered,
+        OVERLAPPED *overlappedBase)
+{
+    const auto overlapped = static_cast<Overlapped *>(overlappedBase);
+    if (overlapped->dptr) {
+        overlapped->dptr->handleNotification(bytesTransfered, errorCode,
+                                             overlappedBase);
+    } else {
+        delete overlapped;
+    }
+}
+
 QT_END_NAMESPACE