Skip to content

Commit af12054

Browse files
authored
Fix a deadlock issue with emscripten_lock_async_acquire() (#25670)
Fix a deadlock issue with emscripten_lock_async_acquire() if user attempted to synchronously acquire the lock right after asynchronously acquiring it. b25abd5#r168975511
1 parent 6f6313a commit af12054

File tree

4 files changed

+41
-11
lines changed

4 files changed

+41
-11
lines changed

src/lib/libwasm_worker.js

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -300,24 +300,21 @@ if (ENVIRONMENT_IS_WASM_WORKER
300300

301301
emscripten_lock_async_acquire__deps: ['$polyfillWaitAsync'],
302302
emscripten_lock_async_acquire: (lock, asyncWaitFinished, userData, maxWaitMilliseconds) => {
303-
let dispatch = (val, ret) => {
304-
setTimeout(() => {
305-
{{{ makeDynCall('vpiip', 'asyncWaitFinished') }}}(lock, val, /*waitResult=*/ret, userData);
306-
}, 0);
307-
};
308303
let tryAcquireLock = () => {
309304
do {
310305
var val = Atomics.compareExchange(HEAP32, {{{ getHeapOffset('lock', 'i32') }}}, 0/*zero represents lock being free*/, 1/*one represents lock being acquired*/);
311-
if (!val) return dispatch(0, 0/*'ok'*/);
306+
if (!val) return {{{ makeDynCall('vpiip', 'asyncWaitFinished') }}}(lock, 0, 0/*'ok'*/, userData);
312307
var wait = Atomics.waitAsync(HEAP32, {{{ getHeapOffset('lock', 'i32') }}}, val, maxWaitMilliseconds);
313308
} while (wait.value === 'not-equal');
314309
#if ASSERTIONS
315310
assert(wait.async || wait.value === 'timed-out');
316311
#endif
317312
if (wait.async) wait.value.then(tryAcquireLock);
318-
else dispatch(val, 2/*'timed-out'*/);
313+
else return {{{ makeDynCall('vpiip', 'asyncWaitFinished') }}}(lock, val, 2/*'timed-out'*/, userData);
319314
};
320-
tryAcquireLock();
315+
// Asynchronously dispatch acquiring the lock so that we have uniform control flow in both
316+
// cases when the lock is acquired, and when it needs to wait.
317+
setTimeout(tryAcquireLock);
321318
},
322319

323320
emscripten_semaphore_async_acquire__deps: ['$polyfillWaitAsync'],

system/include/emscripten/wasm_worker.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,12 @@ void emscripten_lock_busyspin_waitinf_acquire(emscripten_lock_t *lock __attribut
185185
// timeout parameter as int64 nanosecond units, this function takes in the wait
186186
// timeout parameter as double millisecond units. See
187187
// https://github.com/WebAssembly/threads/issues/175 for more information.
188-
// NOTE: This function can be called in both main thread and in Workers. If you
189-
// use this API in Worker, you cannot utilise an infinite loop programming
190-
// model.
188+
// NOTE: This function can be called in both main thread and in Workers.
189+
// NOTE 2: This function will always acquire the lock asynchronously. That is,
190+
// the lock will only be attempted to acquire after current control flow
191+
// yields back to the browser, so that the Wasm call stack is empty.
192+
// This is to guarantee an uniform control flow. If you use this API in
193+
// a Worker, you cannot utilise an infinite loop programming model.
191194
void emscripten_lock_async_acquire(emscripten_lock_t *lock __attribute__((nonnull)),
192195
emscripten_async_wait_volatile_callback_t asyncWaitFinished __attribute__((nonnull)),
193196
void *userData,

test/test_browser.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5317,6 +5317,11 @@ def test_wasm_worker_lock_wait2(self):
53175317
def test_wasm_worker_lock_async_acquire(self):
53185318
self.btest_exit('wasm_worker/lock_async_acquire.c', cflags=['--closure=1', '-sWASM_WORKERS'])
53195319

5320+
# Tests emscripten_lock_async_acquire() function when lock is acquired both synchronously and asynchronously.
5321+
@also_with_minimal_runtime
5322+
def test_wasm_worker_lock_async_and_sync_acquire(self):
5323+
self.btest_exit('wasm_worker/lock_async_and_sync_acquire.c', cflags=['-sWASM_WORKERS'])
5324+
53205325
# Tests emscripten_lock_busyspin_wait_acquire() in Worker and main thread.
53215326
@also_with_minimal_runtime
53225327
def test_wasm_worker_lock_busyspin_wait(self):
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#include <emscripten/wasm_worker.h>
2+
#include <emscripten/threading.h>
3+
#include <stdio.h>
4+
5+
emscripten_lock_t lock = EMSCRIPTEN_LOCK_T_STATIC_INITIALIZER;
6+
7+
void on_acquire(volatile void* address, uint32_t value,
8+
ATOMICS_WAIT_RESULT_T waitResult, void* userData) {
9+
printf("on_acquire: releasing lock.\n");
10+
emscripten_lock_release(&lock);
11+
printf("on_acquire: released lock.\n");
12+
exit(0);
13+
}
14+
15+
int main() {
16+
printf("main: async acquiring lock.\n");
17+
emscripten_lock_async_acquire(&lock, on_acquire, 0, 100);
18+
printf("main: busy-spin acquiring lock.\n");
19+
emscripten_lock_busyspin_waitinf_acquire(&lock);
20+
printf("main: lock acquired.\n");
21+
emscripten_lock_release(&lock);
22+
printf("main: lock released.\n");
23+
emscripten_exit_with_live_runtime();
24+
return 1;
25+
}

0 commit comments

Comments
 (0)