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>
This commit is contained in:
Lorn Potter 2020-10-27 14:32:12 +10:00
parent 107a8058ae
commit 75b0e8d599
2 changed files with 40 additions and 46 deletions

View File

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

View File

@ -137,8 +137,6 @@ public:
QSharedPointer<QRingBuffer> outgoingDataBuffer; QSharedPointer<QRingBuffer> outgoingDataBuffer;
QByteArray requestData; QByteArray requestData;
void doAbort() const;
static void downloadProgress(emscripten_fetch_t *fetch); static void downloadProgress(emscripten_fetch_t *fetch);
static void downloadFailed(emscripten_fetch_t *fetch); static void downloadFailed(emscripten_fetch_t *fetch);
static void downloadSucceeded(emscripten_fetch_t *fetch); static void downloadSucceeded(emscripten_fetch_t *fetch);