wasm: fix network crash on abort

Also work around emscripten bug #8238

emcripten_fetch_close() does not abort the network
request, but instead just free’s the emscripten_fetch_t
object. onsuccess or onerror will still be called, but
now with a stale pointer to the deleted emscripten_fetch_t
object.

See https://github.com/emscripten-core/emscripten/issues/8234

Work around this by setting the userData to null when
we want to abort or are done with the request. The
onerror and onsuccess callbacks can then check this
field (on the the still valid emscripten_fetch_t), and
bail out on the (from the Qt side) aborted request.

Call emcripten_fetch_close() from on error and onsuccess;
this should be the point when the emscripten request
is done, and there will be no more callbacks.

Pick-to: 5.15
Fixes: QTBUG-87813
Change-Id: Ie9b8a29037eb150c23741683588b0f0bfd5d8c63
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>

2 files changed, 40 insertions, 46 deletions

diff --git a/src/network/access/qnetworkreplywasmimpl.cpp b/src/network/access/qnetworkreplywasmimpl.cpp
index 0fc5bbf5a8..2cae6847b8 100644
--- a/src/network/access/qnetworkreplywasmimpl.cpp
+++ b/src/network/access/qnetworkreplywasmimpl.cpp
@@ -67,10 +67,6 @@ QNetworkReplyWasmImplPrivate::QNetworkReplyWasmImplPrivate()
 
 QNetworkReplyWasmImplPrivate::~QNetworkReplyWasmImplPrivate()
 {
-    if (m_fetch) {
-        emscripten_fetch_close(m_fetch);
-        m_fetch = 0;
-    }
 }
 
 QNetworkReplyWasmImpl::QNetworkReplyWasmImpl(QObject *parent)
@@ -115,12 +111,14 @@ void QNetworkReplyWasmImpl::close()
 
 void QNetworkReplyWasmImpl::abort()
 {
-    Q_D( QNetworkReplyWasmImpl);
+    Q_D(QNetworkReplyWasmImpl);
     if (d->state == QNetworkReplyPrivate::Finished || d->state == QNetworkReplyPrivate::Aborted)
         return;
 
     d->state = QNetworkReplyPrivate::Aborted;
-    d->doAbort();
+    d->m_fetch->userData = nullptr;
+
+    d->emitReplyError(QNetworkReply::OperationCanceledError, QStringLiteral("Operation canceled"));
     close();
 }
 
@@ -198,11 +196,6 @@ void QNetworkReplyWasmImplPrivate::setReplyAttributes(quintptr data, int statusC
         handler->q_func()->setAttribute(QNetworkRequest::HttpReasonPhraseAttribute, statusReason);
 }
 
-void QNetworkReplyWasmImplPrivate::doAbort() const
-{
-    emscripten_fetch_close(m_fetch);
-}
-
 constexpr int getArraySize (int factor) {
     return 2 * factor + 1;
 }
@@ -283,7 +276,6 @@ void QNetworkReplyWasmImplPrivate::emitReplyError(QNetworkReply::NetworkError er
 
     q->setError(errorCode, errorString);
     emit q->errorOccurred(errorCode);
-    emit q->finished();
 }
 
 void QNetworkReplyWasmImplPrivate::emitDataReadProgress(qint64 bytesReceived, qint64 bytesTotal)
@@ -316,6 +308,12 @@ void QNetworkReplyWasmImplPrivate::dataReceived(const QByteArray &buffer, int bu
     downloadBuffer.append(buffer, bufferSize);
 
     emit q->readyRead();
+
+    if (downloadBufferCurrentSize == totalDownloadSize) {
+        q->setFinished(true);
+        emit q->readChannelFinished();
+        emit q->finished();
+    }
 }
 
 //taken from qnetworkrequest.cpp
@@ -447,16 +445,14 @@ void QNetworkReplyWasmImplPrivate::_q_bufferOutgoingData()
 
 void QNetworkReplyWasmImplPrivate::downloadSucceeded(emscripten_fetch_t *fetch)
 {
-    QNetworkReplyWasmImplPrivate *reply =
-            reinterpret_cast<QNetworkReplyWasmImplPrivate*>(fetch->userData);
-    if (reply) {
-        QByteArray buffer(fetch->data, fetch->numBytes);
-        reply->dataReceived(buffer, buffer.size());
+    auto reply = reinterpret_cast<QNetworkReplyWasmImplPrivate*>(fetch->userData);
+    if (!reply || reply->state == QNetworkReplyPrivate::Aborted)
+        return;
+    QByteArray buffer(fetch->data, fetch->numBytes);
+    reply->dataReceived(buffer, buffer.size());
 
-        QByteArray statusText(fetch->statusText);
-        reply->setStatusCode(fetch->status, statusText);
-        reply->setReplyFinished();
-    }
+    emscripten_fetch_close(fetch);
+    reply->m_fetch = nullptr;
 }
 
 void QNetworkReplyWasmImplPrivate::setStatusCode(int status, const QByteArray &statusText)
@@ -466,32 +462,29 @@ void QNetworkReplyWasmImplPrivate::setStatusCode(int status, const QByteArray &s
     q->setAttribute(QNetworkRequest::HttpReasonPhraseAttribute, statusText);
 }
 
-void QNetworkReplyWasmImplPrivate::setReplyFinished()
-{
-    Q_Q(QNetworkReplyWasmImpl);
-    q->setFinished(true);
-    emit q->readChannelFinished();
-    emit q->finished();
-}
-
 void QNetworkReplyWasmImplPrivate::stateChange(emscripten_fetch_t *fetch)
 {
-    if (fetch->readyState == /*HEADERS_RECEIVED*/ 2) {
-        size_t headerLength = emscripten_fetch_get_response_headers_length(fetch);
-        QByteArray str(headerLength, Qt::Uninitialized);
-        emscripten_fetch_get_response_headers(fetch, str.data(), str.size());
-        QNetworkReplyWasmImplPrivate *reply =
-                reinterpret_cast<QNetworkReplyWasmImplPrivate*>(fetch->userData);
-        reply->headersReceived(str);
+    if (fetch) {
+        if (!quintptr(fetch->userData))
+            return;
+        auto reply = reinterpret_cast<QNetworkReplyWasmImplPrivate*>(fetch->userData);
+        if (reply->state != QNetworkReplyPrivate::Aborted) {
+            if (fetch->readyState == /*HEADERS_RECEIVED*/ 2) {
+                size_t headerLength = emscripten_fetch_get_response_headers_length(fetch);
+                QByteArray str(headerLength, Qt::Uninitialized);
+                emscripten_fetch_get_response_headers(fetch, str.data(), str.size());
+
+                reply->headersReceived(str);
+            }
+        }
     }
 }
 
 void QNetworkReplyWasmImplPrivate::downloadProgress(emscripten_fetch_t *fetch)
 {
-    QNetworkReplyWasmImplPrivate *reply =
-            reinterpret_cast<QNetworkReplyWasmImplPrivate*>(fetch->userData);
-    Q_ASSERT(reply);
-
+    auto reply = reinterpret_cast<QNetworkReplyWasmImplPrivate*>(fetch->userData);
+    if (!reply || reply->state == QNetworkReplyPrivate::Aborted)
+        return;
     if (fetch->status < 400) {
         uint64_t bytes = fetch->dataOffset + fetch->numBytes;
         uint64_t tBytes = fetch->totalBytes; // totalBytes can be 0 if server not reporting content length
@@ -503,10 +496,13 @@ void QNetworkReplyWasmImplPrivate::downloadProgress(emscripten_fetch_t *fetch)
 
 void QNetworkReplyWasmImplPrivate::downloadFailed(emscripten_fetch_t *fetch)
 {
-    QNetworkReplyWasmImplPrivate *reply = reinterpret_cast<QNetworkReplyWasmImplPrivate*>(fetch->userData);
+
+    auto reply = reinterpret_cast<QNetworkReplyWasmImplPrivate*>(fetch->userData);
+
     if (reply) {
+
         QString reasonStr;
-        if (fetch->status > 600 ||  reply->state == QNetworkReplyPrivate::Aborted)
+        if (fetch->status > 600)
             reasonStr = QStringLiteral("Operation canceled");
         else
             reasonStr = QString::fromUtf8(fetch->statusText);
@@ -516,8 +512,8 @@ void QNetworkReplyWasmImplPrivate::downloadFailed(emscripten_fetch_t *fetch)
         reply->emitReplyError(reply->statusCodeFromHttp(fetch->status, reply->request.url()), reasonStr);
     }
 
-    if (fetch->status >= 400)
-        emscripten_fetch_close(fetch); // Also free data on failure.
+    emscripten_fetch_close(fetch);
+    reply->m_fetch = nullptr;
 }
 
 //taken from qhttpthreaddelegate.cpp
diff --git a/src/network/access/qnetworkreplywasmimpl_p.h b/src/network/access/qnetworkreplywasmimpl_p.h
index 53e9bc3375..db9fd5657e 100644
--- a/src/network/access/qnetworkreplywasmimpl_p.h
+++ b/src/network/access/qnetworkreplywasmimpl_p.h
@@ -137,8 +137,6 @@ public:
     QSharedPointer<QRingBuffer> outgoingDataBuffer;
     QByteArray requestData;
 
-    void doAbort() const;
-
     static void downloadProgress(emscripten_fetch_t *fetch);
     static void downloadFailed(emscripten_fetch_t *fetch);
     static void downloadSucceeded(emscripten_fetch_t *fetch);