From 3e89f8d598a677f004cb273dd2c3cc03d68f32eb Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 24 Apr 2009 10:07:00 +0200 Subject: rewrite QLocalServer native Windows implementation. this makes it much less arcane and buggy, specifically it resolves the internal thread-unsafety. Reviewed-by: thiago --- src/network/socket/qlocalserver.cpp | 6 +- src/network/socket/qlocalserver.h | 4 +- src/network/socket/qlocalserver_p.h | 59 ++------- src/network/socket/qlocalserver_win.cpp | 221 ++++++++++++++------------------ 4 files changed, 112 insertions(+), 178 deletions(-) diff --git a/src/network/socket/qlocalserver.cpp b/src/network/socket/qlocalserver.cpp index d6b1507..1815d73 100644 --- a/src/network/socket/qlocalserver.cpp +++ b/src/network/socket/qlocalserver.cpp @@ -276,9 +276,11 @@ QLocalSocket *QLocalServer::nextPendingConnection() if (d->pendingConnections.isEmpty()) return 0; QLocalSocket *nextSocket = d->pendingConnections.dequeue(); + if (d->pendingConnections.size() <= d->maxPendingConnections) #ifndef Q_OS_WIN - d->socketNotifier->setEnabled(d->pendingConnections.size() - <= d->maxPendingConnections); + d->socketNotifier->setEnabled(true); +#else + d->connectionEventNotifier->setEnabled(true); #endif return nextSocket; } diff --git a/src/network/socket/qlocalserver.h b/src/network/socket/qlocalserver.h index 8e8babd..c745ccb 100644 --- a/src/network/socket/qlocalserver.h +++ b/src/network/socket/qlocalserver.h @@ -89,9 +89,7 @@ private: #if defined(QT_LOCALSOCKET_TCP) Q_PRIVATE_SLOT(d_func(), void _q_onNewConnection()) #elif defined(Q_OS_WIN) - Q_PRIVATE_SLOT(d_func(), void _q_openSocket(HANDLE handle)) - Q_PRIVATE_SLOT(d_func(), void _q_stoppedListening()) - Q_PRIVATE_SLOT(d_func(), void _q_setError(QAbstractSocket::SocketError error, const QString &errorString)) + Q_PRIVATE_SLOT(d_func(), void _q_onNewConnection()) #else Q_PRIVATE_SLOT(d_func(), void _q_socketActivated()) #endif diff --git a/src/network/socket/qlocalserver_p.h b/src/network/socket/qlocalserver_p.h index 8e96401..9af4a20 100644 --- a/src/network/socket/qlocalserver_p.h +++ b/src/network/socket/qlocalserver_p.h @@ -63,7 +63,7 @@ # include #elif defined(Q_OS_WIN) # include -# include +# include #else # include # include @@ -71,52 +71,13 @@ QT_BEGIN_NAMESPACE -#if defined(Q_OS_WIN) && !defined(QT_LOCALSOCKET_TCP) - -/*! - \internal - QLocalServerThread exists because Windows does not have a - way to provide notifications when there is a new connections to - the server. - */ -class QLocalServerThread : public QThread -{ - Q_OBJECT - -Q_SIGNALS: - void connected(HANDLE newSocket); - void error(QAbstractSocket::SocketError error, const QString &errorString); - -public: - QLocalServerThread(QObject *parent = 0); - ~QLocalServerThread(); - void closeServer(); - -public: - QString setName(const QString &name); - void run(); - void stop(); - bool makeHandle(); - - HANDLE gotConnectionEvent; - QQueue pendingHandles; - int maxPendingConnections; -private: - HANDLE stopEvent; - QString fullServerName; -}; - -#endif - class QLocalServerPrivate : public QObjectPrivate { Q_DECLARE_PUBLIC(QLocalServer) public: QLocalServerPrivate() : -#if defined(Q_OS_WIN) && !defined(QT_LOCALSOCKET_TCP) - inWaitingFunction(false), -#elif !defined(QT_LOCALSOCKET_TCP) +#if !defined(QT_LOCALSOCKET_TCP) && !defined(Q_OS_WIN) listenSocket(-1), socketNotifier(0), #endif maxPendingConnections(30), error(QAbstractSocket::UnknownSocketError) @@ -135,12 +96,18 @@ public: QTcpServer tcpServer; QMap socketMap; #elif defined(Q_OS_WIN) - void _q_openSocket(HANDLE socket); - void _q_stoppedListening(); - void _q_setError(QAbstractSocket::SocketError error, const QString &errorString); + struct Listener { + HANDLE handle; + OVERLAPPED overlapped; + }; + + void setError(const QString &function); + bool addListener(); + void _q_onNewConnection(); - QLocalServerThread waitForConnection; - bool inWaitingFunction; + QList listeners; + HANDLE eventHandle; + QWinEventNotifier *connectionEventNotifier; #else void setError(const QString &function); void _q_socketActivated(); diff --git a/src/network/socket/qlocalserver_win.cpp b/src/network/socket/qlocalserver_win.cpp index 880cd7e..b14bbf7 100644 --- a/src/network/socket/qlocalserver_win.cpp +++ b/src/network/socket/qlocalserver_win.cpp @@ -44,68 +44,26 @@ #include "qlocalsocket.h" #include -#include -#include -#include // The buffer size need to be 0 otherwise data could be // lost if the socket that has written data closes the connection // before it is read. Pipewriter is used for write buffering. #define BUFSIZE 0 -QT_BEGIN_NAMESPACE - -QLocalServerThread::QLocalServerThread(QObject *parent) : QThread(parent), - maxPendingConnections(1) -{ - stopEvent = CreateEvent(NULL, TRUE, FALSE, NULL); - gotConnectionEvent = CreateEvent(NULL, TRUE, FALSE, NULL); -} +// ###: This should be a property. Should replace the insane 50 on unix as well. +#define SYSTEM_MAX_PENDING_SOCKETS 8 -QLocalServerThread::~QLocalServerThread() -{ - stop(); - closeServer(); - CloseHandle(stopEvent); - CloseHandle(gotConnectionEvent); -} - -void QLocalServerThread::stop() -{ - if (isRunning()) { - SetEvent(stopEvent); - wait(); - ResetEvent(stopEvent); - } -} - -void QLocalServerThread::closeServer() -{ - while (!pendingHandles.isEmpty()) - CloseHandle(pendingHandles.dequeue()); -} - -QString QLocalServerThread::setName(const QString &name) -{ - QString pipePath = QLatin1String("\\\\.\\pipe\\"); - if (name.startsWith(pipePath)) - fullServerName = name; - else - fullServerName = pipePath + name; - for (int i = pendingHandles.count(); i < maxPendingConnections; ++i) - if (!makeHandle()) - break; - return fullServerName; -} +QT_BEGIN_NAMESPACE -bool QLocalServerThread::makeHandle() +bool QLocalServerPrivate::addListener() { - if (pendingHandles.count() >= maxPendingConnections) - return false; + // The object must not change its address once the + // contained OVERLAPPED struct is passed to Windows. + listeners << Listener(); + Listener &listener = listeners.last(); - HANDLE handle = INVALID_HANDLE_VALUE; QT_WA({ - handle = CreateNamedPipeW( + listener.handle = CreateNamedPipeW( (TCHAR*)fullServerName.utf16(), // pipe name PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED, // read/write access PIPE_TYPE_MESSAGE | // message type pipe @@ -117,7 +75,7 @@ bool QLocalServerThread::makeHandle() 3000, // client time-out NULL); }, { - handle = CreateNamedPipeA( + listener.handle = CreateNamedPipeA( fullServerName.toLocal8Bit().constData(), // pipe name PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED, // read/write access PIPE_TYPE_MESSAGE | // message type pipe @@ -129,68 +87,43 @@ bool QLocalServerThread::makeHandle() 3000, // client time-out NULL); }); - - if (INVALID_HANDLE_VALUE == handle) { + if (listener.handle == INVALID_HANDLE_VALUE) { + setError(QLatin1String("QLocalServerPrivate::addListener")); + listeners.removeLast(); return false; } - pendingHandles.enqueue(handle); + + memset(&listener.overlapped, 0, sizeof(listener.overlapped)); + listener.overlapped.hEvent = eventHandle; + if (!ConnectNamedPipe(listener.handle, &listener.overlapped)) { + switch (GetLastError()) { + case ERROR_IO_PENDING: + break; + case ERROR_PIPE_CONNECTED: + SetEvent(eventHandle); + break; + default: + CloseHandle(listener.handle); + setError(QLatin1String("QLocalServerPrivate::addListener")); + listeners.removeLast(); + return false; + } + } else { + Q_ASSERT_X(false, "QLocalServerPrivate::addListener", "The impossible happened"); + SetEvent(eventHandle); + } return true; } -void QLocalServerThread::run() +void QLocalServerPrivate::setError(const QString &function) { - OVERLAPPED op; - HANDLE handleArray[2]; - memset(&op, 0, sizeof(op)); - handleArray[0] = op.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); - handleArray[1] = stopEvent; - HANDLE handle = INVALID_HANDLE_VALUE; - - forever { - if (INVALID_HANDLE_VALUE == handle) { - makeHandle(); - if (!pendingHandles.isEmpty()) - handle = pendingHandles.dequeue(); - } - if (INVALID_HANDLE_VALUE == handle) { - int windowsError = GetLastError(); - QString function = QLatin1String("QLocalServer::run"); - QString errorString = QLocalServer::tr("%1: Unknown error %2").arg(function).arg(windowsError); - emit error(QAbstractSocket::UnknownSocketError, errorString); - CloseHandle(handleArray[0]); - SetEvent(gotConnectionEvent); - return; - } - - BOOL isConnected = ConnectNamedPipe(handle, &op) ? TRUE : (GetLastError() == ERROR_PIPE_CONNECTED); - if (!isConnected) { - switch (WaitForMultipleObjects(2, handleArray, FALSE, INFINITE)) - { - case WAIT_OBJECT_0 + 1: - CloseHandle(handle); - CloseHandle(handleArray[0]); - return; - } - } - emit connected(handle); - handle = INVALID_HANDLE_VALUE; - ResetEvent(handleArray[0]); - SetEvent(gotConnectionEvent); - } + int windowsError = GetLastError(); + errorString = QString::fromLatin1("%1: %2").arg(function).arg(qt_error_string(windowsError)); + error = QAbstractSocket::UnknownSocketError; } void QLocalServerPrivate::init() { - Q_Q(QLocalServer); - qRegisterMetaType("HANDLE"); - q->connect(&waitForConnection, SIGNAL(connected(HANDLE)), - q, SLOT(_q_openSocket(HANDLE)), Qt::QueuedConnection); - q->connect(&waitForConnection, SIGNAL(finished()), - q, SLOT(_q_stoppedListening()), Qt::QueuedConnection); - q->connect(&waitForConnection, SIGNAL(terminated()), - q, SLOT(_q_stoppedListening()), Qt::QueuedConnection); - q->connect(&waitForConnection, SIGNAL(error(QAbstractSocket::SocketError, const QString &)), - q, SLOT(_q_setError(QAbstractSocket::SocketError, const QString &))); } bool QLocalServerPrivate::removeServer(const QString &name) @@ -201,35 +134,71 @@ bool QLocalServerPrivate::removeServer(const QString &name) bool QLocalServerPrivate::listen(const QString &name) { - fullServerName = waitForConnection.setName(name); - serverName = name; - waitForConnection.start(); - return true; -} + Q_Q(QLocalServer); -void QLocalServerPrivate::_q_setError(QAbstractSocket::SocketError e, const QString &eString) -{ - error = e; - errorString = eString; -} + QString pipePath = QLatin1String("\\\\.\\pipe\\"); + if (name.startsWith(pipePath)) + fullServerName = name; + else + fullServerName = pipePath + name; -void QLocalServerPrivate::_q_stoppedListening() -{ - Q_Q(QLocalServer); - if (!inWaitingFunction) - q->close(); + // Use only one event for all listeners of one socket. + // The idea is that listener events are rare, so polling all listeners once in a while is + // cheap compared to waiting for N additional events in each iteration of the main loop. + eventHandle = CreateEvent(NULL, TRUE, FALSE, NULL); + connectionEventNotifier = new QWinEventNotifier(eventHandle , q); + q->connect(connectionEventNotifier, SIGNAL(activated(HANDLE)), q, SLOT(_q_onNewConnection())); + + for (int i = 0; i < SYSTEM_MAX_PENDING_SOCKETS; ++i) + if (!addListener()) + return false; + return true; } -void QLocalServerPrivate::_q_openSocket(HANDLE handle) +void QLocalServerPrivate::_q_onNewConnection() { Q_Q(QLocalServer); - q->incomingConnection((int)handle); + 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 (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) { + setError(QLatin1String("QLocalServerPrivate::_q_onNewConnection")); + closeServer(); + return; + } + + ++i; + } + } } void QLocalServerPrivate::closeServer() { - waitForConnection.stop(); - waitForConnection.closeServer(); + connectionEventNotifier->setEnabled(false); // Otherwise, closed handle is checked before deleter runs + connectionEventNotifier->deleteLater(); + connectionEventNotifier = 0; + CloseHandle(eventHandle); + for (int i = 0; i < listeners.size(); ++i) + CloseHandle(listeners[i].handle); + listeners.clear(); } void QLocalServerPrivate::waitForNewConnection(int msecs, bool *timedOut) @@ -238,14 +207,12 @@ void QLocalServerPrivate::waitForNewConnection(int msecs, bool *timedOut) if (!pendingConnections.isEmpty() || !q->isListening()) return; - DWORD result = WaitForSingleObject(waitForConnection.gotConnectionEvent, - (msecs == -1) ? INFINITE : msecs); + DWORD result = WaitForSingleObject(eventHandle, (msecs == -1) ? INFINITE : msecs); if (result == WAIT_TIMEOUT) { if (timedOut) *timedOut = true; } else { - ResetEvent(waitForConnection.gotConnectionEvent); - QCoreApplication::instance()->processEvents(); + _q_onNewConnection(); } } -- cgit v0.12