From f31a90c9ea50cfa462a6c70f897c6d68a966f263 Mon Sep 17 00:00:00 2001 From: Anthony Pesch Date: Sat, 14 Feb 2026 23:51:57 +0100 Subject: [PATCH] fetch / fix memory leak inside saveResponseAndStatus when using streaming mode when streaming was enabled - * onprogress allocated emscripten_fetch_t.data * onload called saveResponseAndStatus * xhr.response was NULL (due to streaming) which caused ptr to remain 0 * emscripten_fetch_t.data was uncondtionally overwritten by ptr, leaking what was allocated during onprogress --- src/Fetch.js | 44 ++++++++++++++++----------- src/lib/libfetch.js | 1 + test/fetch/test_fetch_stream_file.cpp | 1 - 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/Fetch.js b/src/Fetch.js index 5d0fd81c38af5..6f5a0a4a3647c 100644 --- a/src/Fetch.js +++ b/src/Fetch.js @@ -576,21 +576,23 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) { // This receives a condition, which determines whether to save the xhr's // response, or just 0. function saveResponseAndStatus() { - var ptr = 0; - var ptrLen = 0; - if (xhr.response && fetchAttrLoadToMemory && {{{ makeGetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, '*') }}} === 0) { + let ptrLen = 0; + if (fetchAttrLoadToMemory && xhr.response) { + let ptr = {{{ makeGetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, '*') }}}; ptrLen = xhr.response.byteLength; - } - if (ptrLen > 0) { -#if FETCH_DEBUG - dbg(`fetch: allocating ${ptrLen} bytes in Emscripten heap for xhr data`); -#endif + // The data pointer malloc()ed here has the same lifetime as the emscripten_fetch_t structure itself has, and is // freed when emscripten_fetch_close() is called. - ptr = _realloc({{{ makeGetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, '*') }}}, ptrLen); + if (ptrLen > readI53FromI64(fetch + {{{ C_STRUCTS.emscripten_fetch_t.numBytes }}})) { +#if FETCH_DEBUG + dbg(`fetch: allocating ${ptrLen} bytes in Emscripten heap for xhr data`); +#endif + ptr = _realloc(ptr, ptrLen); + {{{ makeSetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, 'ptr', '*') }}} + } + HEAPU8.set(new Uint8Array(/** @type{Array} */(xhr.response)), ptr); } - {{{ makeSetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, 'ptr', '*') }}} writeI53ToI64(fetch + {{{ C_STRUCTS.emscripten_fetch_t.numBytes }}}, ptrLen); writeI53ToI64(fetch + {{{ C_STRUCTS.emscripten_fetch_t.dataOffset }}}, 0); var len = xhr.response ? xhr.response.byteLength : 0; @@ -660,21 +662,27 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) { if (!Fetch.xhrs.has(id)) { return; } - var ptrLen = (fetchAttrLoadToMemory && fetchAttrStreamData && xhr.response) ? xhr.response.byteLength : 0; - var ptr = 0; - if (ptrLen > 0 && fetchAttrLoadToMemory && fetchAttrStreamData) { -#if FETCH_DEBUG - dbg(`fetch: allocating ${ptrLen} bytes in Emscripten heap for xhr data`); -#endif + let ptrLen = 0; + if (fetchAttrLoadToMemory && fetchAttrStreamData && xhr.response) { + let ptr = {{{ makeGetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, '*') }}}; + ptrLen = xhr.response.byteLength; + #if ASSERTIONS assert(onprogress, 'When doing a streaming fetch, you should have an onprogress handler registered to receive the chunks!'); #endif + // The data pointer malloc()ed here has the same lifetime as the emscripten_fetch_t structure itself has, and is // freed when emscripten_fetch_close() is called. - ptr = _realloc({{{ makeGetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, '*') }}}, ptrLen); + if (ptrLen > readI53FromI64(fetch + {{{ C_STRUCTS.emscripten_fetch_t.numBytes }}})) { +#if FETCH_DEBUG + dbg(`fetch: allocating ${ptrLen} bytes in Emscripten heap for xhr data`); +#endif + ptr = _realloc(ptr, ptrLen); + {{{ makeSetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, 'ptr', '*') }}} + } + HEAPU8.set(new Uint8Array(/** @type{Array} */(xhr.response)), ptr); } - {{{ makeSetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, 'ptr', '*') }}} writeI53ToI64(fetch + {{{ C_STRUCTS.emscripten_fetch_t.numBytes }}}, ptrLen); writeI53ToI64(fetch + {{{ C_STRUCTS.emscripten_fetch_t.dataOffset }}}, e.loaded - ptrLen); writeI53ToI64(fetch + {{{ C_STRUCTS.emscripten_fetch_t.totalBytes }}}, e.total); diff --git a/src/lib/libfetch.js b/src/lib/libfetch.js index 03dba8681e6be..3a9ed46f5ced6 100644 --- a/src/lib/libfetch.js +++ b/src/lib/libfetch.js @@ -33,6 +33,7 @@ var LibraryFetch = { '$Fetch', '$fetchXHR', '$callUserCallback', + '$readI53FromI64', '$writeI53ToI64', '$stringToUTF8', '$stringToNewUTF8', diff --git a/test/fetch/test_fetch_stream_file.cpp b/test/fetch/test_fetch_stream_file.cpp index a31db5b2aa4a6..68ffc90ad7636 100644 --- a/test/fetch/test_fetch_stream_file.cpp +++ b/test/fetch/test_fetch_stream_file.cpp @@ -20,7 +20,6 @@ int main() { attr.onsuccess = [](emscripten_fetch_t *fetch) { printf("Finished downloading %llu bytes\n", fetch->totalBytes); printf("Data checksum: %08X\n", checksum); - assert(fetch->data == 0); // The data was streamed via onprogress, no bytes available here. assert(fetch->numBytes == 0); assert(fetch->totalBytes == 134217728); assert(checksum == 0xA7F8E858U);