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>
bb10
Joerg Bornemann 2016-03-10 14:07:16 +01:00
parent 8589bb334f
commit ac8dae8b5e
2 changed files with 35 additions and 27 deletions

View File

@ -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;
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);
// 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);
// 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();
addListener();
if (pendingConnections.size() > maxPendingConnections)
connectionEventNotifier->setEnabled(false);
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;
// 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()

View File

@ -1,2 +0,0 @@
[processConnection:1 client]
windows