Ensure we don't move the Listener struct around

We're passing a pointer into the Listener struct to
Windows API, so ensure we keep that pointer valid even
when our container changes.

Change-Id: I32b8de8cd959ecc7f574063451ed7238b69e7125
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
This commit is contained in:
Lars Knoll 2019-10-29 16:24:41 +01:00
parent bc4e7aecc0
commit 5df1cf38e3
2 changed files with 25 additions and 22 deletions

View File

@ -99,15 +99,18 @@ public:
QMap<quintptr, QTcpSocket*> socketMap; QMap<quintptr, QTcpSocket*> socketMap;
#elif defined(Q_OS_WIN) #elif defined(Q_OS_WIN)
struct Listener { struct Listener {
HANDLE handle; Listener() = default;
HANDLE handle = nullptr;
OVERLAPPED overlapped; OVERLAPPED overlapped;
bool connected; bool connected = false;
private:
Q_DISABLE_COPY(Listener)
}; };
void setError(const QString &function); void setError(const QString &function);
bool addListener(); bool addListener();
QList<Listener> listeners; std::vector<std::unique_ptr<Listener>> listeners;
HANDLE eventHandle; HANDLE eventHandle;
QWinEventNotifier *connectionEventNotifier; QWinEventNotifier *connectionEventNotifier;
#else #else

View File

@ -62,8 +62,8 @@ bool QLocalServerPrivate::addListener()
{ {
// The object must not change its address once the // The object must not change its address once the
// contained OVERLAPPED struct is passed to Windows. // contained OVERLAPPED struct is passed to Windows.
listeners << Listener(); listeners.push_back(std::make_unique<Listener>());
Listener &listener = listeners.last(); auto &listener = listeners.back();
SECURITY_ATTRIBUTES sa; SECURITY_ATTRIBUTES sa;
sa.nLength = sizeof(SECURITY_ATTRIBUTES); sa.nLength = sizeof(SECURITY_ATTRIBUTES);
@ -175,7 +175,7 @@ bool QLocalServerPrivate::addListener()
sa.lpSecurityDescriptor = pSD.data(); sa.lpSecurityDescriptor = pSD.data();
} }
listener.handle = CreateNamedPipe( listener->handle = CreateNamedPipe(
reinterpret_cast<const wchar_t *>(fullServerName.utf16()), // pipe name reinterpret_cast<const wchar_t *>(fullServerName.utf16()), // pipe name
PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED, // read/write access PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED, // read/write access
PIPE_TYPE_BYTE | // byte type pipe PIPE_TYPE_BYTE | // byte type pipe
@ -187,32 +187,32 @@ bool QLocalServerPrivate::addListener()
3000, // client time-out 3000, // client time-out
&sa); &sa);
if (listener.handle == INVALID_HANDLE_VALUE) { if (listener->handle == INVALID_HANDLE_VALUE) {
setError(QLatin1String("QLocalServerPrivate::addListener")); setError(QLatin1String("QLocalServerPrivate::addListener"));
listeners.removeLast(); listeners.pop_back();
return false; return false;
} }
if (worldSID) if (worldSID)
FreeSid(worldSID); FreeSid(worldSID);
memset(&listener.overlapped, 0, sizeof(listener.overlapped)); memset(&listener->overlapped, 0, sizeof(OVERLAPPED));
listener.overlapped.hEvent = eventHandle; listener->overlapped.hEvent = eventHandle;
// Beware! ConnectNamedPipe will reset the eventHandle to non-signaled. // Beware! ConnectNamedPipe will reset the eventHandle to non-signaled.
// Callers of addListener must check all listeners for connections. // Callers of addListener must check all listeners for connections.
if (!ConnectNamedPipe(listener.handle, &listener.overlapped)) { if (!ConnectNamedPipe(listener->handle, &listener->overlapped)) {
switch (GetLastError()) { switch (GetLastError()) {
case ERROR_IO_PENDING: case ERROR_IO_PENDING:
listener.connected = false; listener->connected = false;
break; break;
case ERROR_PIPE_CONNECTED: case ERROR_PIPE_CONNECTED:
listener.connected = true; listener->connected = true;
break; break;
default: default:
CloseHandle(listener.handle); CloseHandle(listener->handle);
setError(QLatin1String("QLocalServerPrivate::addListener")); setError(QLatin1String("QLocalServerPrivate::addListener"));
listeners.removeLast(); listeners.pop_back();
return false; return false;
} }
} else { } else {
@ -284,12 +284,12 @@ void QLocalServerPrivate::_q_onNewConnection()
// Testing shows that there is indeed absolutely no guarantee which listener gets // 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. // a client connection first, so there is no way around polling all of them.
for (int i = 0; i < listeners.size(); ) { for (size_t i = 0; i < listeners.size(); ) {
HANDLE handle = listeners[i].handle; HANDLE handle = listeners[i]->handle;
if (listeners[i].connected if (listeners[i]->connected
|| GetOverlappedResult(handle, &listeners[i].overlapped, &dummy, FALSE)) || GetOverlappedResult(handle, &listeners[i]->overlapped, &dummy, FALSE))
{ {
listeners.removeAt(i); listeners.erase(listeners.begin() + i);
addListener(); addListener();
@ -319,8 +319,8 @@ void QLocalServerPrivate::closeServer()
connectionEventNotifier->deleteLater(); connectionEventNotifier->deleteLater();
connectionEventNotifier = 0; connectionEventNotifier = 0;
CloseHandle(eventHandle); CloseHandle(eventHandle);
for (int i = 0; i < listeners.size(); ++i) for (size_t i = 0; i < listeners.size(); ++i)
CloseHandle(listeners[i].handle); CloseHandle(listeners[i]->handle);
listeners.clear(); listeners.clear();
} }