From ac8dae8b5eafa07704155dd636d8da2ec21d1c6f Mon Sep 17 00:00:00 2001
From: Joerg Bornemann <joerg.bornemann@theqtcompany.com>
Date: Thu, 10 Mar 2016 14:07:16 +0100
Subject: [PATCH] QLocalServer/Win: Fix race condition in listen().
Suppose a client connects while the QLocalServer is still in the loop
that calls addListener. The connection would SetEvent(eventHandle),
but every call to ConnectNamedPipe would ResetEvent(eventHandle).
Thus, the connection is never detected by the notifier on eventHandle.
Callers of addListener must check the connection state of every
listener to make sure that no client connected while setting up
listeners.
Task-number: QTBUG-49254
Change-Id: Ia961927ea76973708e6e3f73510695eb5d6a0e4c
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>
---
src/network/socket/qlocalserver_win.cpp | 70 +++++++++++--------
.../network/socket/qlocalsocket/BLACKLIST | 2 -
2 files changed, 40 insertions(+), 32 deletions(-)
delete mode 100644 tests/auto/network/socket/qlocalsocket/BLACKLIST
diff --git a/src/network/socket/qlocalserver_win.cpp b/src/network/socket/qlocalserver_win.cpp
index 265f8949725..06ad62f5481 100644
--- a/src/network/socket/qlocalserver_win.cpp
+++ b/src/network/socket/qlocalserver_win.cpp
@@ -192,6 +192,9 @@ bool QLocalServerPrivate::addListener()
memset(&listener.overlapped, 0, sizeof(listener.overlapped));
listener.overlapped.hEvent = eventHandle;
+
+ // Beware! ConnectNamedPipe will reset the eventHandle to non-signaled.
+ // Callers of addListener must check all listeners for connections.
if (!ConnectNamedPipe(listener.handle, &listener.overlapped)) {
switch (GetLastError()) {
case ERROR_IO_PENDING:
@@ -199,7 +202,6 @@ bool QLocalServerPrivate::addListener()
break;
case ERROR_PIPE_CONNECTED:
listener.connected = true;
- SetEvent(eventHandle);
break;
default:
CloseHandle(listener.handle);
@@ -251,6 +253,8 @@ bool QLocalServerPrivate::listen(const QString &name)
for (int i = 0; i < SYSTEM_MAX_PENDING_SOCKETS; ++i)
if (!addListener())
return false;
+
+ _q_onNewConnection();
return true;
}
@@ -264,37 +268,43 @@ void QLocalServerPrivate::_q_onNewConnection()
{
Q_Q(QLocalServer);
DWORD dummy;
-
- // Reset first, otherwise we could reset an event which was asserted
- // immediately after we checked the conn status.
- ResetEvent(eventHandle);
-
- // Testing shows that there is indeed absolutely no guarantee which listener gets
- // a client connection first, so there is no way around polling all of them.
- for (int i = 0; i < listeners.size(); ) {
- HANDLE handle = listeners[i].handle;
- if (listeners[i].connected
- || GetOverlappedResult(handle, &listeners[i].overlapped, &dummy, FALSE))
- {
- listeners.removeAt(i);
-
- addListener();
-
- if (pendingConnections.size() > maxPendingConnections)
- connectionEventNotifier->setEnabled(false);
-
- // Make this the last thing so connected slots can wreak the least havoc
- q->incomingConnection((quintptr)handle);
- } else {
- if (GetLastError() != ERROR_IO_INCOMPLETE) {
- q->close();
- setError(QLatin1String("QLocalServerPrivate::_q_onNewConnection"));
- return;
+ bool tryAgain;
+ do {
+ tryAgain = false;
+
+ // Reset first, otherwise we could reset an event which was asserted
+ // immediately after we checked the conn status.
+ ResetEvent(eventHandle);
+
+ // Testing shows that there is indeed absolutely no guarantee which listener gets
+ // a client connection first, so there is no way around polling all of them.
+ for (int i = 0; i < listeners.size(); ) {
+ HANDLE handle = listeners[i].handle;
+ if (listeners[i].connected
+ || GetOverlappedResult(handle, &listeners[i].overlapped, &dummy, FALSE))
+ {
+ listeners.removeAt(i);
+
+ addListener();
+
+ if (pendingConnections.size() > maxPendingConnections)
+ connectionEventNotifier->setEnabled(false);
+ else
+ tryAgain = true;
+
+ // Make this the last thing so connected slots can wreak the least havoc
+ q->incomingConnection((quintptr)handle);
+ } else {
+ if (GetLastError() != ERROR_IO_INCOMPLETE) {
+ q->close();
+ setError(QLatin1String("QLocalServerPrivate::_q_onNewConnection"));
+ return;
+ }
+
+ ++i;
}
-
- ++i;
}
- }
+ } while (tryAgain);
}
void QLocalServerPrivate::closeServer()
diff --git a/tests/auto/network/socket/qlocalsocket/BLACKLIST b/tests/auto/network/socket/qlocalsocket/BLACKLIST
deleted file mode 100644
index 11ddef30a58..00000000000
--- a/tests/auto/network/socket/qlocalsocket/BLACKLIST
+++ /dev/null
@@ -1,2 +0,0 @@
-[processConnection:1 client]
-windows
--
GitLab