diff options
author | Maurice Kalinowski <maurice.kalinowski@qt.io> | 2016-05-13 08:40:39 +0200 |
---|---|---|
committer | Maurice Kalinowski <maurice.kalinowski@qt.io> | 2016-05-17 09:58:42 +0000 |
commit | 8714c99f65381af194cc4991c4258fec4b9a6307 (patch) | |
tree | 8aa67f138f23442b22a5c80880fb052e2fb5762f | |
parent | 6e928460bc92d7d67a5b95790225142a65de32fa (diff) |
winrt: Fix potential crash when reading closed sockets
Using multiple concurrent requests can cause a delay between a socket
closing and getting deleted. At that point the state was closingDown,
but not wasDeleted yet. Especially on slower arm devices, callbacks are
done from another thread causing synchronization issues.
Hence closingDown needs to be synced and handleReadyRead needs to have
more criterias to return early to avoid invalid access crashes.
Easiest to reproduce is heavy scrolling on the mapviewer example when it
downloads a huge amount of tiles and cancels those requests when not in
view anymore.
Change-Id: I442b6243bbefb3af938b6b1b3739a6a85b4887c0
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>
-rw-r--r-- | src/network/socket/qnativesocketengine_winrt.cpp | 30 | ||||
-rw-r--r-- | src/network/socket/qnativesocketengine_winrt_p.h | 2 |
2 files changed, 20 insertions, 12 deletions
diff --git a/src/network/socket/qnativesocketengine_winrt.cpp b/src/network/socket/qnativesocketengine_winrt.cpp index 599f37929c..d79dffa215 100644 --- a/src/network/socket/qnativesocketengine_winrt.cpp +++ b/src/network/socket/qnativesocketengine_winrt.cpp @@ -458,16 +458,21 @@ void QNativeSocketEngine::close() } #if _MSC_VER >= 1900 - // To close the connection properly (not with a hard reset) all pending read operation have to - // be finished or cancelled. The API isn't available on Windows 8.1 though. - ComPtr<IStreamSocket3> socket3; - hr = d->tcpSocket()->QueryInterface(IID_PPV_ARGS(&socket3)); - Q_ASSERT_SUCCEEDED(hr); + hr = QEventDispatcherWinRT::runOnXamlThread([d]() { + HRESULT hr; + // To close the connection properly (not with a hard reset) all pending read operation have to + // be finished or cancelled. The API isn't available on Windows 8.1 though. + ComPtr<IStreamSocket3> socket3; + hr = d->tcpSocket()->QueryInterface(IID_PPV_ARGS(&socket3)); + Q_ASSERT_SUCCEEDED(hr); - ComPtr<IAsyncAction> action; - hr = socket3->CancelIOAsync(&action); - Q_ASSERT_SUCCEEDED(hr); - hr = QWinRTFunctions::await(action); + ComPtr<IAsyncAction> action; + hr = socket3->CancelIOAsync(&action); + Q_ASSERT_SUCCEEDED(hr); + hr = QWinRTFunctions::await(action); + Q_ASSERT_SUCCEEDED(hr); + return S_OK; + }); Q_ASSERT_SUCCEEDED(hr); #endif // _MSC_VER >= 1900 @@ -1261,9 +1266,12 @@ void QNativeSocketEnginePrivate::handleConnectionEstablished(IAsyncAction *actio HRESULT QNativeSocketEnginePrivate::handleReadyRead(IAsyncBufferOperation *asyncInfo, AsyncStatus status) { - Q_Q(QNativeSocketEngine); - if (wasDeleted || isDeletingChildren) + if (closingDown || wasDeleted || isDeletingChildren + || socketState == QAbstractSocket::UnconnectedState) { return S_OK; + } + + Q_Q(QNativeSocketEngine); // A read in UnconnectedState will close the socket and return -1 and thus tell the caller, // that the connection was closed. The socket cannot be closed here, as the subsequent read diff --git a/src/network/socket/qnativesocketengine_winrt_p.h b/src/network/socket/qnativesocketengine_winrt_p.h index 4a5bc81769..2c1c42ecbc 100644 --- a/src/network/socket/qnativesocketengine_winrt_p.h +++ b/src/network/socket/qnativesocketengine_winrt_p.h @@ -148,7 +148,7 @@ public: qintptr socketDescriptor; bool notifyOnRead, notifyOnWrite, notifyOnException; - bool closingDown; + QAtomicInt closingDown; enum ErrorString { NonBlockingInitFailedErrorString, |