Skip to content

Commit 05221e7

Browse files
authored
Support some synchronous Fetch API calls with the wasm backend, without a fetch-worker (#9490)
@dschuff looked into this and found that we can only use the fetch-worker to help with synchronous fetches when we are in a worker, since the main thread can't block on another one. But if we are in a worker, then we can just do a sync xhr directly! Thanks to that we can enable test_fetch_sync_xhr_in_wasm, so this helps #7024, but it does not fix all the cases. There is still emscripten_fetch_wait, and I believe some IDB operations may also not work. I noticed that some fetch test coverage may be misleading here, as we skip browser tests that use WASM=0 with pthreads (since wasm2js doesn't support that); I added an explicit skip to the test for emscripten_fetch_wait to make things clearer. This gets rid of a block of code starting with Should we try IndexedDB first?. That code seems unnecessary: It checks if we will need IDB and then errors if we will but it isn't ready. However, the actual operation later would error on missing IDB anyhow, so this is redundant. Worse, it looks wrong to me, as it says a simple fetch should use IDB but that doesn't seem right - in particular, it made the test here fail. Regardless, just removing the code seems best.
1 parent 09e9050 commit 05221e7

File tree

5 files changed

+33
-23
lines changed

5 files changed

+33
-23
lines changed

emcc.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,6 +1265,16 @@ def is_supported_link_flag(f):
12651265
if shared.Settings.USE_PTHREADS:
12661266
shared.Settings.FETCH_WORKER_FILE = unsuffixed(os.path.basename(target)) + '.fetch.js'
12671267

1268+
if shared.Settings.FETCH:
1269+
# In asm.js+pthreads we can use a fetch worker, which is made from the main
1270+
# asm.js code. That lets us do sync operations by blocking on the worker etc.
1271+
# In the wasm backend we don't have a fetch worker implemented yet, however,
1272+
# we can still do basic synchronous fetches in the same places: if we can
1273+
# block on another thread then we aren't the main thread, and if we aren't
1274+
# the main thread then synchronous xhrs are legitimate.
1275+
if shared.Settings.USE_PTHREADS and not shared.Settings.WASM_BACKEND:
1276+
shared.Settings.USE_FETCH_WORKER = 1
1277+
12681278
if shared.Settings.DEMANGLE_SUPPORT:
12691279
shared.Settings.EXPORTED_FUNCTIONS += ['___cxa_demangle']
12701280
forced_stdlibs.append('libc++abi')
@@ -2358,11 +2368,8 @@ def repl(m):
23582368
f.write(shared.read_and_preprocess(shared.path_from_root('src', 'worker.js'), expand_macros=True))
23592369

23602370
# Generate the fetch.js worker script for multithreaded emscripten_fetch() support if targeting pthreads.
2361-
if shared.Settings.FETCH and shared.Settings.USE_PTHREADS:
2362-
if shared.Settings.WASM_BACKEND:
2363-
logger.warning('Bug/TODO: Blocking calls to the fetch API do not currently work under WASM backend (https://github.com/emscripten-core/emscripten/issues/7024)')
2364-
else:
2365-
shared.make_fetch_worker(final, shared.Settings.FETCH_WORKER_FILE)
2371+
if shared.Settings.USE_FETCH_WORKER:
2372+
shared.make_fetch_worker(final, shared.Settings.FETCH_WORKER_FILE)
23662373

23672374
# exit block 'memory initializer'
23682375
log_time('memory initializer')

src/Fetch.js

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ var Fetch = {
4242
},
4343
#endif
4444

45-
#if USE_PTHREADS
45+
#if USE_FETCH_WORKER
4646
initFetchWorker: function() {
4747
var stackSize = 128*1024;
4848
var stack = allocate(stackSize>>2, "i32*", ALLOC_DYNAMIC);
@@ -51,7 +51,7 @@ var Fetch = {
5151
#endif
5252

5353
staticInit: function() {
54-
#if USE_PTHREADS
54+
#if USE_FETCH_WORKER
5555
var isMainThread = (typeof ENVIRONMENT_IS_FETCH_WORKER === 'undefined' && !ENVIRONMENT_IS_PTHREAD);
5656
#else
5757
var isMainThread = (typeof ENVIRONMENT_IS_FETCH_WORKER === 'undefined');
@@ -65,7 +65,7 @@ var Fetch = {
6565
Fetch.dbInstance = db;
6666

6767
if (isMainThread) {
68-
#if USE_PTHREADS
68+
#if USE_FETCH_WORKER
6969
Fetch.initFetchWorker();
7070
#endif
7171
removeRunDependency('library_fetch_init');
@@ -78,7 +78,7 @@ var Fetch = {
7878
Fetch.dbInstance = false;
7979

8080
if (isMainThread) {
81-
#if USE_PTHREADS
81+
#if USE_FETCH_WORKER
8282
Fetch.initFetchWorker();
8383
#endif
8484
removeRunDependency('library_fetch_init');
@@ -87,7 +87,7 @@ var Fetch = {
8787
Fetch.openDatabase('emscripten_filesystem', 1, onsuccess, onerror);
8888
#endif // ~FETCH_SUPPORT_INDEXEDDB
8989

90-
#if USE_PTHREADS
90+
#if USE_FETCH_WORKER
9191
if (isMainThread) {
9292
#if FETCH_SUPPORT_INDEXEDDB
9393
addRunDependency('library_fetch_init');
@@ -543,16 +543,6 @@ function emscripten_start_fetch(fetch, successcb, errorcb, progresscb, readystat
543543
__emscripten_fetch_xhr(fetch, cacheResultAndReportSuccess, reportError, reportProgress, reportReadyStateChange);
544544
};
545545

546-
// Should we try IndexedDB first?
547-
var needsIndexedDbConnection = !fetchAttrReplace || requestMethod === 'EM_IDB_STORE' || requestMethod === 'EM_IDB_DELETE';
548-
if (needsIndexedDbConnection && !Fetch.dbInstance) {
549-
#if FETCH_DEBUG
550-
console.error('fetch: failed to read IndexedDB! Database is not open.');
551-
#endif
552-
reportError(fetch, 0, 'IndexedDB is not open');
553-
return 0; // todo: free
554-
}
555-
556546
if (requestMethod === 'EM_IDB_STORE') {
557547
// TODO(?): Here we perform a clone of the data, because storing shared typed arrays to IndexedDB does not seem to be allowed.
558548
var ptr = HEAPU32[fetch_attr + {{{ C_STRUCTS.emscripten_fetch_attr_t.requestData }}} >> 2];

src/settings.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,6 +1395,12 @@ var FETCH_DEBUG = 0;
13951395
// If nonzero, enables emscripten_fetch API.
13961396
var FETCH = 0;
13971397

1398+
// Whether to use an asm.js fetch worker when using FETCH. Note that this is
1399+
// only relevant for fastcomp, where we support asm.js. As a result, some
1400+
// synchronous fetch operations that depend on the fetch worker may not work
1401+
// with the wasm backend, like waiting or IndexedDB.
1402+
var USE_FETCH_WORKER = 0;
1403+
13981404
// Internal: name of the file containing the Fetch *.fetch.js, if relevant
13991405
// Do not set yourself.
14001406
var FETCH_WORKER_FILE = '';

system/lib/fetch/emscripten_fetch.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,13 @@ emscripten_fetch_t* emscripten_fetch(emscripten_fetch_attr_t* fetch_attr, const
148148

149149
#undef STRDUP_OR_ABORT
150150

151-
#if __EMSCRIPTEN_PTHREADS__
151+
// In asm.js we can use a fetch worker, which is created from the main asm.js
152+
// code. That lets us do sync operations by blocking on the worker etc.
153+
// In the wasm backend we don't have a fetch worker implemented yet, however,
154+
// we can still do basic synchronous fetches in the same places: if we can
155+
// block on another thread then we aren't the main thread, and if we aren't
156+
// the main thread then synchronous xhrs are legitimate.
157+
#if __EMSCRIPTEN_PTHREADS__ && !defined(__wasm__)
152158
const bool waitable = (fetch_attr->attributes & EMSCRIPTEN_FETCH_WAITABLE) != 0;
153159
// Depending on the type of fetch, we can either perform it in the same Worker/thread than the
154160
// caller, or we might need to run it in a separate Worker. There is a dedicated fetch worker that

tests/test_browser.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4349,10 +4349,10 @@ def test_fetch_implicit_append(self):
43494349
self.btest('fetch/example_synchronous_fetch.cpp', expected='200', args=['-s', 'FETCH=1', '-s', 'WASM=0', '-s', 'USE_PTHREADS=1', '-s', 'PROXY_TO_PTHREAD=1'])
43504350

43514351
# Tests synchronous emscripten_fetch() usage from wasm pthread in fastcomp.
4352-
@no_wasm_backend("fetch API uses an asm.js based web worker to run synchronous XHRs and IDB operations")
4352+
@requires_threads
43534353
def test_fetch_sync_xhr_in_wasm(self):
43544354
shutil.copyfile(path_from_root('tests', 'gears.png'), 'gears.png')
4355-
self.btest('fetch/example_synchronous_fetch.cpp', expected='200', args=['-s', 'FETCH=1', '-s', 'WASM=1', '-s', 'USE_PTHREADS=1', '-s', 'PROXY_TO_PTHREAD=1'])
4355+
self.btest('fetch/example_synchronous_fetch.cpp', expected='200', args=['-s', 'FETCH=1', '-s', 'USE_PTHREADS=1', '-s', 'PROXY_TO_PTHREAD=1'])
43564356

43574357
# Tests that the Fetch API works for synchronous XHRs when used with --proxy-to-worker.
43584358
@requires_threads
@@ -4364,6 +4364,7 @@ def test_fetch_sync_xhr_in_proxy_to_worker(self):
43644364
also_asmjs=True)
43654365

43664366
# Tests waiting on EMSCRIPTEN_FETCH_WAITABLE request from a worker thread
4367+
@no_wasm_backend("emscripten_fetch_wait uses an asm.js based web worker")
43674368
@requires_threads
43684369
def test_fetch_sync_fetch_in_main_thread(self):
43694370
shutil.copyfile(path_from_root('tests', 'gears.png'), 'gears.png')

0 commit comments

Comments
 (0)