Skip to content

Commit e33466c

Browse files
authored
[fetch] Fix double free in fetch_free() with streaming fetch requests (#25856)
`fetch->data` will be allocated and freed in `onprogress` if fetch attribute `EMSCRIPTEN_FETCH_STREAM_DATA` is used, however it will still be freed in `fetch_free()` causing double free problem.
1 parent 2f01ef4 commit e33466c

File tree

4 files changed

+62
-11
lines changed

4 files changed

+62
-11
lines changed

src/Fetch.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) {
576576
#endif
577577
// The data pointer malloc()ed here has the same lifetime as the emscripten_fetch_t structure itself has, and is
578578
// freed when emscripten_fetch_close() is called.
579-
ptr = _malloc(ptrLen);
579+
ptr = _realloc({{{ makeGetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, '*') }}}, ptrLen);
580580
HEAPU8.set(new Uint8Array(/** @type{Array<number>} */(xhr.response)), ptr);
581581
}
582582
{{{ makeSetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, 'ptr', '*') }}}
@@ -653,8 +653,9 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) {
653653
#if ASSERTIONS
654654
assert(onprogress, 'When doing a streaming fetch, you should have an onprogress handler registered to receive the chunks!');
655655
#endif
656-
// Allocate byte data in Emscripten heap for the streamed memory block (freed immediately after onprogress call)
657-
ptr = _malloc(ptrLen);
656+
// The data pointer malloc()ed here has the same lifetime as the emscripten_fetch_t structure itself has, and is
657+
// freed when emscripten_fetch_close() is called.
658+
ptr = _realloc({{{ makeGetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, '*') }}}, ptrLen);
658659
HEAPU8.set(new Uint8Array(/** @type{Array<number>} */(xhr.response)), ptr);
659660
}
660661
{{{ makeSetValue('fetch', C_STRUCTS.emscripten_fetch_t.data, 'ptr', '*') }}}
@@ -668,7 +669,6 @@ function fetchXHR(fetch, onsuccess, onerror, onprogress, onreadystatechange) {
668669
{{{ makeSetValue('fetch', C_STRUCTS.emscripten_fetch_t.status, 'status', 'i16') }}}
669670
if (xhr.statusText) stringToUTF8(xhr.statusText, fetch + {{{ C_STRUCTS.emscripten_fetch_t.statusText }}}, 64);
670671
onprogress(fetch, e);
671-
_free(ptr);
672672
};
673673
xhr.onreadystatechange = (e) => {
674674
// check if xhr was aborted by user and don't try to call back

src/lib/libfetch.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ var LibraryFetch = {
2929
emscripten_start_fetch: startFetch,
3030
emscripten_start_fetch__deps: [
3131
'malloc',
32-
'free',
32+
'realloc',
3333
'$Fetch',
3434
'$fetchXHR',
3535
'$callUserCallback',
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright 2025 The Emscripten Authors. All rights reserved.
2+
// Emscripten is available under two separate licenses, the MIT license and the
3+
// University of Illinois/NCSA Open Source License. Both these licenses can be
4+
// found in the LICENSE file.
5+
6+
#include <string.h>
7+
#include <stdlib.h>
8+
#include <stdio.h>
9+
#include <assert.h>
10+
#include <emscripten.h>
11+
#include <emscripten/fetch.h>
12+
#include <emscripten/eventloop.h>
13+
14+
int main() {
15+
emscripten_fetch_attr_t attr;
16+
emscripten_fetch_attr_init(&attr);
17+
strcpy(attr.requestMethod, "GET");
18+
attr.onsuccess = [](emscripten_fetch_t *fetch) {
19+
printf("Finished downloading %llu bytes\n", fetch->totalBytes);
20+
emscripten_fetch_close(fetch);
21+
};
22+
attr.onerror = [](emscripten_fetch_t *fetch) {
23+
printf("Downloading failed with status code: %d.\n", fetch->status);
24+
if (fetch->status != (uint16_t)-1) { // if not aborted with emscripten_fetch_close()
25+
emscripten_fetch_close(fetch);
26+
}
27+
};
28+
attr.onprogress = [](emscripten_fetch_t *fetch) {
29+
printf("Downloading.. %.2f%s complete. Received chunk [%llu, %llu[\n",
30+
(fetch->totalBytes > 0) ? ((fetch->dataOffset + fetch->numBytes) * 100.0 / fetch->totalBytes) : (double)(fetch->dataOffset + fetch->numBytes),
31+
(fetch->totalBytes > 0) ? "%" : " bytes",
32+
fetch->dataOffset,
33+
fetch->dataOffset + fetch->numBytes);
34+
35+
emscripten_set_immediate([](void *arg) {
36+
emscripten_fetch_t *fetch = (emscripten_fetch_t *)arg;
37+
printf("Abort fetch when downloading\n");
38+
emscripten_fetch_close(fetch);
39+
}, fetch);
40+
};
41+
attr.attributes = EMSCRIPTEN_FETCH_LOAD_TO_MEMORY | EMSCRIPTEN_FETCH_STREAM_DATA;
42+
emscripten_fetch(&attr, "largefile.txt");
43+
}

test/test_browser.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4460,18 +4460,21 @@ def test_fetch_response_headers(self, args):
44604460
shutil.copy(test_file('gears.png'), '.')
44614461
self.btest_exit('fetch/test_fetch_response_headers.cpp', cflags=['-sFETCH_DEBUG', '-sFETCH', '-pthread', '-sPROXY_TO_PTHREAD'] + args)
44624462

4463-
# Test emscripten_fetch() usage to stream a fetch in to memory without storing the full file in memory
4464-
# Streaming only works the fetch backend.
4465-
@also_with_wasm2js
4466-
def test_fetch_stream_file(self):
4467-
# Strategy: create a large 128MB file, and compile with a small 16MB Emscripten heap, so that the tested file
4468-
# won't fully fit in the heap. This verifies that streaming works properly.
4463+
def make_largefile(self):
44694464
s = '12345678'
44704465
for _ in range(14):
44714466
s = s[::-1] + s # length of str will be 2^17=128KB
44724467
with open('largefile.txt', 'w') as f:
44734468
for _ in range(1024):
44744469
f.write(s)
4470+
4471+
# Test emscripten_fetch() usage to stream a fetch in to memory without storing the full file in memory
4472+
# Streaming only works the fetch backend.
4473+
@also_with_wasm2js
4474+
def test_fetch_stream_file(self):
4475+
# Strategy: create a large 128MB file, and compile with a small 16MB Emscripten heap, so that the tested file
4476+
# won't fully fit in the heap. This verifies that streaming works properly.
4477+
self.make_largefile()
44754478
self.btest_exit('fetch/test_fetch_stream_file.cpp', cflags=['-sFETCH_DEBUG', '-sFETCH', '-sFETCH_STREAMING'])
44764479

44774480
@also_with_fetch_streaming
@@ -4538,6 +4541,11 @@ def test_fetch_stream_async(self):
45384541
create_file('myfile.dat', 'hello world\n' * 1000)
45394542
self.btest_exit('fetch/test_fetch_stream_async.c', cflags=['-sFETCH', '-sFETCH_STREAMING'])
45404543

4544+
@also_with_asan
4545+
def test_fetch_stream_abort(self):
4546+
self.make_largefile()
4547+
self.btest_exit('fetch/test_fetch_stream_abort.cpp', cflags=['-sFETCH', '-sFETCH_STREAMING', '-sALLOW_MEMORY_GROWTH'])
4548+
45414549
@also_with_fetch_streaming
45424550
def test_fetch_persist(self):
45434551
create_file('myfile.dat', 'hello world\n')

0 commit comments

Comments
 (0)