diff options
author | Piotr Wierciński <piotr.wiercinski@qt.io> | 2024-04-02 16:55:07 +0200 |
---|---|---|
committer | Piotr Wierciński <piotr.wiercinski@qt.io> | 2024-05-10 09:55:19 +0000 |
commit | 9907ef0d64f743fbf269967a65005d490ba0a432 (patch) | |
tree | 2b918e0fe6752f6206eef4267d9f0af7a5f94c55 | |
parent | 0ae44ccc6f3017213ba8a66d703b59c5fcb336a5 (diff) |
wasm: Allow fetching from background thread
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 <piotr.wiercinski@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
Reviewed-by: Jøger Hansegård <joger.hansegard@qt.io>
-rw-r--r-- | src/network/access/qnetworkreplywasmimpl.cpp | 177 | ||||
-rw-r--r-- | src/network/access/qnetworkreplywasmimpl_p.h | 26 | ||||
-rw-r--r-- | 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 c02f0b4e61..7d2b6a701e 100644 --- a/src/network/access/qnetworkreplywasmimpl.cpp +++ b/src/network/access/qnetworkreplywasmimpl.cpp @@ -9,6 +9,7 @@ #include <QtCore/qcoreapplication.h> #include <QtCore/qfileinfo.h> #include <QtCore/qthread.h> +#include <QtCore/private/qeventdispatcher_wasm_p.h> #include <QtCore/private/qoffsetstringarray_p.h> #include <QtCore/private/qtools_p.h> @@ -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<QByteArray> 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<void *>(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<void *>(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<QByteArray> 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<QNetworkReplyWasmImplPrivate*>(fetch->userData); - if (reply) { + auto fetchContext = static_cast<FetchContext *>(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<QNetworkReplyWasmImplPrivate*>(fetch->userData); + const auto fetchContext = static_cast<FetchContext*>(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<QNetworkReplyWasmImplPrivate*>(fetch->userData); + const auto fetchContext = static_cast<FetchContext*>(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<QNetworkReplyWasmImplPrivate*>(fetch->userData); - if (reply) { + const auto fetchContext = static_cast<FetchContext*>(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 ae167799d7..4b00bb09ea 100644 --- a/src/network/access/qnetworkreplywasmimpl_p.h +++ b/src/network/access/qnetworkreplywasmimpl_p.h @@ -28,6 +28,7 @@ #include <emscripten/fetch.h> #include <memory> +#include <mutex> 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<QRingBuffer> 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 3dcd8dd916..e37316b2db 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(); |