From dcf7da7c93c0d974bfb72ffa677a1d26fb9be5e0 Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Mon, 10 Oct 2016 14:35:19 +0200 Subject: winrt: Do not lose initial data for TCP connections When a client connects and sends data immediately it was possible that initial data was lost as the state was set too late. If the callback was called before the state was set the socket engine just discarded the data. So the state has to be set before the callback is registered. The new implementation needs a list of pending read operations. It can happen that the "readyRead" callback is triggered directly while "put_Completed" is called. The callback reassigns readOp which causes a "function not implemented" exception when it jumps back to the "put_Completed" call in "initialize" Task-number: QTBUG-55889 Change-Id: I5f52e3377b6176f1f90f227ac0bf52b60ee2d95a Reviewed-by: Maurice Kalinowski --- src/network/socket/qnativesocketengine_winrt.cpp | 34 ++++++++++++++++++------ src/network/socket/qnativesocketengine_winrt_p.h | 2 +- 2 files changed, 27 insertions(+), 9 deletions(-) (limited to 'src/network/socket') diff --git a/src/network/socket/qnativesocketengine_winrt.cpp b/src/network/socket/qnativesocketengine_winrt.cpp index bd9b443602..2ff028d2c5 100644 --- a/src/network/socket/qnativesocketengine_winrt.cpp +++ b/src/network/socket/qnativesocketengine_winrt.cpp @@ -317,26 +317,31 @@ bool QNativeSocketEngine::initialize(qintptr socketDescriptor, QAbstractSocket:: // Start processing incoming data if (d->socketType == QAbstractSocket::TcpSocket) { HRESULT hr; - QEventDispatcherWinRT::runOnXamlThread([d, &hr, socket, this]() { + QEventDispatcherWinRT::runOnXamlThread([&hr, socket, socketState, this]() { + Q_D(QNativeSocketEngine); ComPtr buffer; HRESULT hr = g->bufferFactory->Create(READ_BUFFER_SIZE, &buffer); RETURN_OK_IF_FAILED("initialize(): Could not create buffer"); ComPtr stream; hr = socket->get_InputStream(&stream); RETURN_OK_IF_FAILED("initialize(): Could not obtain input stream"); - hr = stream->ReadAsync(buffer.Get(), READ_BUFFER_SIZE, InputStreamOptions_Partial, d->readOp.GetAddressOf()); + ComPtr readOp; + hr = stream->ReadAsync(buffer.Get(), READ_BUFFER_SIZE, InputStreamOptions_Partial, readOp.GetAddressOf()); RETURN_OK_IF_FAILED_WITH_ARGS("initialize(): Failed to read from the socket buffer (%s).", socketDescription(this).constData()); - hr = d->readOp->put_Completed(Callback(d, &QNativeSocketEnginePrivate::handleReadyRead).Get()); + d->pendingReadOps.append(readOp); + d->socketState = socketState; + hr = readOp->put_Completed(Callback(d, &QNativeSocketEnginePrivate::handleReadyRead).Get()); RETURN_OK_IF_FAILED_WITH_ARGS("initialize(): Failed to set socket read callback (%s).", socketDescription(this).constData()); return S_OK; }); if (FAILED(hr)) return false; + } else { + d->socketState = socketState; } - d->socketState = socketState; return true; } @@ -567,9 +572,9 @@ void QNativeSocketEngine::close() } #endif // _MSC_VER >= 1900 - if (d->readOp) { + for (ComPtr readOp : d->pendingReadOps) { ComPtr info; - hr = d->readOp.As(&info); + hr = readOp.As(&info); Q_ASSERT_SUCCEEDED(hr); if (info) { hr = info->Cancel(); @@ -933,9 +938,11 @@ void QNativeSocketEngine::establishRead() hr = g->bufferFactory->Create(READ_BUFFER_SIZE, &buffer); RETURN_HR_IF_FAILED("establishRead(): Failed to create buffer"); - hr = stream->ReadAsync(buffer.Get(), READ_BUFFER_SIZE, InputStreamOptions_Partial, &d->readOp); + ComPtr readOp; + hr = stream->ReadAsync(buffer.Get(), READ_BUFFER_SIZE, InputStreamOptions_Partial, readOp.GetAddressOf()); RETURN_HR_IF_FAILED("establishRead(): Failed to initiate socket read"); - hr = d->readOp->put_Completed(Callback(d, &QNativeSocketEnginePrivate::handleReadyRead).Get()); + d->pendingReadOps.append(readOp); + hr = readOp->put_Completed(Callback(d, &QNativeSocketEnginePrivate::handleReadyRead).Get()); RETURN_HR_IF_FAILED("establishRead(): Failed to register read callback"); return S_OK; }); @@ -1410,7 +1417,15 @@ HRESULT QNativeSocketEnginePrivate::handleReadyRead(IAsyncBufferOperation *async } Q_Q(QNativeSocketEngine); + for (int i = 0; i < pendingReadOps.count(); ++i) { + if (pendingReadOps.at(i).Get() == asyncInfo) { + pendingReadOps.takeAt(i); + break; + } + } + static QMutex mutex; + mutex.lock(); // 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 // might fail then. @@ -1463,6 +1478,7 @@ HRESULT QNativeSocketEnginePrivate::handleReadyRead(IAsyncBufferOperation *async if (notifyOnRead) emit q->readReady(); + mutex.unlock(); hr = QEventDispatcherWinRT::runOnXamlThread([buffer, q, this]() { UINT32 readBufferLength; @@ -1476,12 +1492,14 @@ HRESULT QNativeSocketEnginePrivate::handleReadyRead(IAsyncBufferOperation *async hr = buffer->put_Length(0); RETURN_HR_IF_FAILED("handleReadyRead(): Could not set buffer length"); + ComPtr readOp; hr = stream->ReadAsync(buffer.Get(), readBufferLength, InputStreamOptions_Partial, &readOp); if (FAILED(hr)) { qErrnoWarning(hr, "handleReadyRead(): Could not read into socket stream buffer (%s).", socketDescription(q).constData()); return S_OK; } + pendingReadOps.append(readOp); hr = readOp->put_Completed(Callback(this, &QNativeSocketEnginePrivate::handleReadyRead).Get()); if (FAILED(hr)) { qErrnoWarning(hr, "handleReadyRead(): Failed to set socket read callback (%s).", diff --git a/src/network/socket/qnativesocketengine_winrt_p.h b/src/network/socket/qnativesocketengine_winrt_p.h index 605f3631b9..79530d57f1 100644 --- a/src/network/socket/qnativesocketengine_winrt_p.h +++ b/src/network/socket/qnativesocketengine_winrt_p.h @@ -214,7 +214,7 @@ private: { return reinterpret_cast(socketDescriptor); } Microsoft::WRL::ComPtr tcpListener; Microsoft::WRL::ComPtr connectOp; - Microsoft::WRL::ComPtr> readOp; + QVector>> pendingReadOps; QBuffer readBytes; QMutex readMutex; bool emitOnNewDatagram; -- cgit v1.2.3