From 9907ef0d64f743fbf269967a65005d490ba0a432 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wierci=C5=84ski?= Date: Tue, 2 Apr 2024 16:55:07 +0200 Subject: [PATCH] wasm: Allow fetching from background thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow network request from background thread by proxing it to main thread if needed. Introduce "fetchHelper" which is stored on heap and owns "outgoingData" which must be valid during entire fetch operation. It is also used for synchronization between thread that has scheduled fetch operation and the one that is executing it. Enable the test that was skipped before fix. Fixes: QTBUG-124111 Change-Id: Ifafa4c40fa435122639fa861a61fbf96340a7747 Reviewed-by: Piotr Wierciński Reviewed-by: Mårten Nordheim Reviewed-by: Morten Johan Sørvig Reviewed-by: Jøger Hansegård --- src/network/access/qnetworkreplywasmimpl.cpp | 177 ++++++++++++------- src/network/access/qnetworkreplywasmimpl_p.h | 26 ++- tests/auto/wasm/fetchapi/tst_fetchapi.cpp | 2 - 3 files changed, 143 insertions(+), 62 deletions(-) diff --git a/src/network/access/qnetworkreplywasmimpl.cpp b/src/network/access/qnetworkreplywasmimpl.cpp index c02f0b4e61c..7d2b6a701e5 100644 --- a/src/network/access/qnetworkreplywasmimpl.cpp +++ b/src/network/access/qnetworkreplywasmimpl.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -63,11 +64,27 @@ QNetworkReplyWasmImplPrivate::QNetworkReplyWasmImplPrivate() , totalDownloadSize(0) , percentFinished(0) , m_fetch(nullptr) + , m_fetchContext(nullptr) { } QNetworkReplyWasmImplPrivate::~QNetworkReplyWasmImplPrivate() { + + if (m_fetchContext) { // fetch has been initiated + std::unique_lock lock{ m_fetchContext->mutex }; + + if (m_fetchContext->state == FetchContext::State::SCHEDULED + || m_fetchContext->state == FetchContext::State::SENT + || m_fetchContext->state == FetchContext::State::CANCELED) { + m_fetchContext->reply = nullptr; + m_fetchContext->state = FetchContext::State::TO_BE_DESTROYED; + } else if (m_fetchContext->state == FetchContext::State::FINISHED) { + lock.unlock(); + delete m_fetchContext; + } + } + } QNetworkReplyWasmImpl::QNetworkReplyWasmImpl(QObject *parent) @@ -116,7 +133,7 @@ void QNetworkReplyWasmImpl::close() d->state = QNetworkReplyPrivate::Finished; d->setCanceled(); } - + emscripten_fetch_close(d->m_fetch); QNetworkReply::close(); } @@ -134,8 +151,14 @@ void QNetworkReplyWasmImpl::abort() void QNetworkReplyWasmImplPrivate::setCanceled() { Q_Q(QNetworkReplyWasmImpl); - if (m_fetch) - m_fetch->userData = nullptr; + { + if (m_fetchContext) { + std::scoped_lock lock{ m_fetchContext->mutex }; + if (m_fetchContext->state == FetchContext::State::SCHEDULED + || m_fetchContext->state == FetchContext::State::SENT) + m_fetchContext->state = FetchContext::State::CANCELED; + } + } emitReplyError(QNetworkReply::OperationCanceledError, QStringLiteral("Operation canceled")); q->setFinished(true); @@ -227,48 +250,7 @@ void QNetworkReplyWasmImplPrivate::doSendRequest() emscripten_fetch_attr_t attr; emscripten_fetch_attr_init(&attr); - strcpy(attr.requestMethod, q->methodName().constData()); - - QList headersData = request.rawHeaderList(); - int arrayLength = getArraySize(headersData.count()); - const char *customHeaders[arrayLength]; - QStringList trimmedHeaders; - - if (headersData.count() > 0) { - int i = 0; - for (const auto &headerName : headersData) { - if (isUnsafeHeader(QLatin1StringView(headerName.constData()))) { - trimmedHeaders.push_back(QString::fromLatin1(headerName)); - } else { - customHeaders[i++] = headerName.constData(); - customHeaders[i++] = request.rawHeader(headerName).constData(); - } - } - if (!trimmedHeaders.isEmpty()) { - qWarning() << "Qt has trimmed the following forbidden headers from the request:" - << trimmedHeaders.join(QLatin1StringView(", ")); - } - customHeaders[i] = nullptr; - attr.requestHeaders = customHeaders; - } - - if (outgoingData) { // data from post request - // handle extra data - requestData = outgoingData->readAll(); // is there a size restriction here? - if (!requestData.isEmpty()) { - attr.requestData = requestData.data(); - attr.requestDataSize = requestData.size(); - } - } - - QByteArray userName, password; - // username & password - if (!request.url().userInfo().isEmpty()) { - userName = request.url().userName().toUtf8(); - password = request.url().password().toUtf8(); - attr.userName = userName.constData(); - attr.password = password.constData(); - } + qstrncpy(attr.requestMethod, q->methodName().constData(), 32); // requestMethod is char[32] in emscripten attr.attributes = EMSCRIPTEN_FETCH_LOAD_TO_MEMORY; @@ -293,13 +275,67 @@ void QNetworkReplyWasmImplPrivate::doSendRequest() attr.onprogress = QNetworkReplyWasmImplPrivate::downloadProgress; attr.onreadystatechange = QNetworkReplyWasmImplPrivate::stateChange; attr.timeoutMSecs = request.transferTimeout(); - attr.userData = reinterpret_cast(this); - QString dPath = "/home/web_user/"_L1 + request.url().fileName(); - QByteArray destinationPath = dPath.toUtf8(); - attr.destinationPath = destinationPath.constData(); + m_fetchContext = new FetchContext(this);; + attr.userData = static_cast(m_fetchContext); + if (outgoingData) { // data from post request + m_fetchContext->requestData = outgoingData->readAll(); // is there a size restriction here? + if (!m_fetchContext->requestData.isEmpty()) { + attr.requestData = m_fetchContext->requestData.data(); + attr.requestDataSize = m_fetchContext->requestData.size(); + } + } - m_fetch = emscripten_fetch(&attr, request.url().toString().toUtf8().constData()); + QEventDispatcherWasm::runOnMainThread([attr, fetchContext = m_fetchContext]() mutable { + std::unique_lock lock{ fetchContext->mutex }; + if (fetchContext->state == FetchContext::State::CANCELED) { + fetchContext->state = FetchContext::State::FINISHED; + return; + } else if (fetchContext->state == FetchContext::State::TO_BE_DESTROYED) { + lock.unlock(); + delete fetchContext; + return; + } + const auto reply = fetchContext->reply; + const auto &request = reply->request; + + QByteArray userName, password; + if (!request.url().userInfo().isEmpty()) { + userName = request.url().userName().toUtf8(); + password = request.url().password().toUtf8(); + attr.userName = userName.constData(); + attr.password = password.constData(); + } + + QList headersData = request.rawHeaderList(); + int arrayLength = getArraySize(headersData.count()); + const char *customHeaders[arrayLength]; + QStringList trimmedHeaders; + if (headersData.count() > 0) { + int i = 0; + for (const auto &headerName : headersData) { + if (isUnsafeHeader(QLatin1StringView(headerName.constData()))) { + trimmedHeaders.push_back(QString::fromLatin1(headerName)); + } else { + customHeaders[i++] = headerName.constData(); + customHeaders[i++] = request.rawHeader(headerName).constData(); + } + } + if (!trimmedHeaders.isEmpty()) { + qWarning() << "Qt has trimmed the following forbidden headers from the request:" + << trimmedHeaders.join(QLatin1StringView(", ")); + } + customHeaders[i] = nullptr; + attr.requestHeaders = customHeaders; + } + + auto url = request.url().toString().toUtf8(); + QString dPath = "/home/web_user/"_L1 + request.url().fileName(); + QByteArray destinationPath = dPath.toUtf8(); + attr.destinationPath = destinationPath.constData(); + reply->m_fetch = emscripten_fetch(&attr, url.constData()); + fetchContext->state = FetchContext::State::SENT; + }); state = Working; } @@ -477,8 +513,18 @@ void QNetworkReplyWasmImplPrivate::_q_bufferOutgoingData() void QNetworkReplyWasmImplPrivate::downloadSucceeded(emscripten_fetch_t *fetch) { - auto reply = reinterpret_cast(fetch->userData); - if (reply) { + auto fetchContext = static_cast(fetch->userData); + std::unique_lock lock{ fetchContext->mutex }; + + if (fetchContext->state == FetchContext::State::TO_BE_DESTROYED) { + lock.unlock(); + delete fetchContext; + return; + } else if (fetchContext->state == FetchContext::State::CANCELED) { + fetchContext->state = FetchContext::State::FINISHED; + return; + } else if (fetchContext->state == FetchContext::State::SENT) { + const auto reply = fetchContext->reply; if (reply->state != QNetworkReplyPrivate::Aborted) { QByteArray buffer(fetch->data, fetch->numBytes); reply->dataReceived(buffer); @@ -487,8 +533,8 @@ void QNetworkReplyWasmImplPrivate::downloadSucceeded(emscripten_fetch_t *fetch) reply->setReplyFinished(); } reply->m_fetch = nullptr; + fetchContext->state = FetchContext::State::FINISHED; } - emscripten_fetch_close(fetch); } void QNetworkReplyWasmImplPrivate::setReplyFinished() @@ -509,7 +555,8 @@ void QNetworkReplyWasmImplPrivate::setStatusCode(int status, const QByteArray &s void QNetworkReplyWasmImplPrivate::stateChange(emscripten_fetch_t *fetch) { - auto reply = reinterpret_cast(fetch->userData); + const auto fetchContext = static_cast(fetch->userData); + const auto reply = fetchContext->reply; if (reply && reply->state != QNetworkReplyPrivate::Aborted) { if (fetch->readyState == /*HEADERS_RECEIVED*/ 2) { size_t headerLength = emscripten_fetch_get_response_headers_length(fetch); @@ -522,7 +569,8 @@ void QNetworkReplyWasmImplPrivate::stateChange(emscripten_fetch_t *fetch) void QNetworkReplyWasmImplPrivate::downloadProgress(emscripten_fetch_t *fetch) { - auto reply = reinterpret_cast(fetch->userData); + const auto fetchContext = static_cast(fetch->userData); + const auto reply = fetchContext->reply; if (reply && reply->state != QNetworkReplyPrivate::Aborted) { if (fetch->status < 400) { uint64_t bytes = fetch->dataOffset + fetch->numBytes; @@ -536,8 +584,18 @@ void QNetworkReplyWasmImplPrivate::downloadProgress(emscripten_fetch_t *fetch) void QNetworkReplyWasmImplPrivate::downloadFailed(emscripten_fetch_t *fetch) { - auto reply = reinterpret_cast(fetch->userData); - if (reply) { + const auto fetchContext = static_cast(fetch->userData); + std::unique_lock lock{ fetchContext->mutex }; + + if (fetchContext->state == FetchContext::State::TO_BE_DESTROYED) { + lock.unlock(); + delete fetchContext; + return; + } else if (fetchContext->state == FetchContext::State::CANCELED) { + fetchContext->state = FetchContext::State::FINISHED; + return; + } else if (fetchContext->state == FetchContext::State::SENT) { + const auto reply = fetchContext->reply; if (reply->state != QNetworkReplyPrivate::Aborted) { QString reasonStr; if (fetch->status > 600) @@ -548,12 +606,13 @@ void QNetworkReplyWasmImplPrivate::downloadFailed(emscripten_fetch_t *fetch) reply->dataReceived(buffer); QByteArray statusText(fetch->statusText); reply->setStatusCode(fetch->status, statusText); - reply->emitReplyError(reply->statusCodeFromHttp(fetch->status, reply->request.url()), reasonStr); + reply->emitReplyError(reply->statusCodeFromHttp(fetch->status, reply->request.url()), + reasonStr); reply->setReplyFinished(); } reply->m_fetch = nullptr; + fetchContext->state = FetchContext::State::FINISHED; } - emscripten_fetch_close(fetch); } //taken from qhttpthreaddelegate.cpp diff --git a/src/network/access/qnetworkreplywasmimpl_p.h b/src/network/access/qnetworkreplywasmimpl_p.h index ae167799d73..4b00bb09eaa 100644 --- a/src/network/access/qnetworkreplywasmimpl_p.h +++ b/src/network/access/qnetworkreplywasmimpl_p.h @@ -28,6 +28,7 @@ #include #include +#include QT_BEGIN_NAMESPACE @@ -63,6 +64,29 @@ private: QByteArray methodName() const; }; +class QNetworkReplyWasmImplPrivate; + +/*! + The FetchContext class ensures the requestData object remains valid + while a fetch operation is pending. Since Emscripten fetch is asynchronous, + requestData must persist until one of the final callbacks is invoked. + Additionally, there's a potential race condition between the thread + scheduling the fetch operation and the one executing it. Since fetch must + occur on the main thread due to browser limitations, + a mutex safeguards the FetchContext to ensure atomic state transitions. +*/ +struct FetchContext +{ + enum class State { SCHEDULED, SENT, FINISHED, CANCELED, TO_BE_DESTROYED }; + + FetchContext(QNetworkReplyWasmImplPrivate *networkReply) : reply(networkReply) { } + + QNetworkReplyWasmImplPrivate *reply{ nullptr }; + std::mutex mutex; + QByteArray requestData; + State state{ State::SCHEDULED }; +}; + class QNetworkReplyWasmImplPrivate: public QNetworkReplyPrivate { public: @@ -101,7 +125,6 @@ public: QIODevice *outgoingData; std::shared_ptr outgoingDataBuffer; - QByteArray requestData; static void downloadProgress(emscripten_fetch_t *fetch); static void downloadFailed(emscripten_fetch_t *fetch); @@ -111,6 +134,7 @@ public: static QNetworkReply::NetworkError statusCodeFromHttp(int httpStatusCode, const QUrl &url); emscripten_fetch_t *m_fetch; + FetchContext *m_fetchContext; void setReplyFinished(); void setCanceled(); diff --git a/tests/auto/wasm/fetchapi/tst_fetchapi.cpp b/tests/auto/wasm/fetchapi/tst_fetchapi.cpp index 3dcd8dd9165..e37316b2db3 100644 --- a/tests/auto/wasm/fetchapi/tst_fetchapi.cpp +++ b/tests/auto/wasm/fetchapi/tst_fetchapi.cpp @@ -69,11 +69,9 @@ void tst_FetchApi::sendRequestOnMainThread() void tst_FetchApi::sendRequestOnBackgroundThread() { - QSKIP("Skip this test until we fix fetching from background threads."); QEventLoop mainEventLoop; BackgroundThread *backgroundThread = new BackgroundThread(); connect(backgroundThread, &BackgroundThread::finished, &mainEventLoop, &QEventLoop::quit); - connect(backgroundThread, &BackgroundThread::finished, backgroundThread, &QObject::deleteLater); backgroundThread->start(); mainEventLoop.exec();