Detect nested usage of callUserCallback. NFC Detect cases where `callUserCallback` is called while already running user code. This doesn't change the behaviour, but warns about (currently) incorrect usage of `callUserCallback`. Split out from #17441 which fixes a real bug in glfw where `callUserCallback` could cause the runtime to exit mid-program. This bug only effects programs that are built with `EXIT_RUNTIME` but its serious enough we probably want to look at more robust solution. For now, issuing a warning seems like a good first step.
diff --git a/src/library.js b/src/library.js index 2ebdd0c..c15cce6 100644 --- a/src/library.js +++ b/src/library.js
@@ -3502,6 +3502,12 @@ return; } try { +#if ASSERTIONS + if (userCodeEntriesOnStack != 0) { + warnOnce('nested call to callUserCallback detected (This should only be used to enter from the event loop)'); + } + userCodeEntriesOnStack += 1; +#endif func(); #if EXIT_RUNTIME || USE_PTHREADS #if USE_PTHREADS && !EXIT_RUNTIME @@ -3512,6 +3518,11 @@ } catch (e) { handleException(e); } +#if ASSERTIONS + finally { + userCodeEntriesOnStack -= 1; + } +#endif }, $maybeExit__deps: ['exit', '$handleException',
diff --git a/src/postamble.js b/src/postamble.js index 777164c..42d141b 100644 --- a/src/postamble.js +++ b/src/postamble.js
@@ -161,6 +161,11 @@ abortWrapperDepth += 2; #endif +#if ASSERTIONS + // TODO(sbc): Have `callMain` use `callUserCallback` to avoid duplicating + // this tracking. + userCodeEntriesOnStack += 1; +#endif #if STANDALONE_WASM entryFunction(); // _start (in crt1.c) will call exit() if main return non-zero. So we know @@ -189,6 +194,9 @@ return handleException(e); #endif // !PROXY_TO_PTHREAD } finally { +#if ASSERTIONS + userCodeEntriesOnStack -= 1; +#endif calledMain = true; #if ABORT_ON_WASM_EXCEPTIONS
diff --git a/src/preamble.js b/src/preamble.js index d374cbb..3e52012 100644 --- a/src/preamble.js +++ b/src/preamble.js
@@ -368,6 +368,11 @@ var dependenciesFulfilled = null; // overridden to take different actions when all run dependencies are fulfilled #if ASSERTIONS var runDependencyTracking = {}; +// Keeps track of how many times we have entered user code in the current stack. +// For example, this starts of at 0, and is set to 1 when we enter main. This +// enables callUserCallback to issue a warning when it is called from within +// user code (its designed to be used to enter user code from the event loop). +var userCodeEntriesOnStack = 0; #endif function getUniqueRunDependency(id) {
diff --git a/tests/other/test_nested_user_callback.c b/tests/other/test_nested_user_callback.c new file mode 100644 index 0000000..053a65e --- /dev/null +++ b/tests/other/test_nested_user_callback.c
@@ -0,0 +1,36 @@ +#include <assert.h> +#include <emscripten.h> +#include <stdlib.h> +#include <stdio.h> + +void myatexit() { + puts("myatexit\n"); +} + +int main() { + atexit(myatexit); + + puts("in main"); + + EM_ASM({ + // Without this Push, the runtime will exit after the user callback. + runtimeKeepalivePush(); + // Should generate a warning because we are already within user code. + callUserCallback(() => out("in user callback")); + runtimeKeepalivePop(); + + assert(!runtimeExited); + out('before: ' + runtimeExited); + callUserCallback(() => out("in user callback; without runtimeKeepalivePush")); + // callUserCallback without runtimeKeepalivePush will result in the runtime + // exiting at the end of the user callback. + // So we should never get here: + assert(false); + }); + + puts("returning from main"); + // Should never get here since the EM_ASM block above causes application to + // exit. + assert(0); + return 0; +}
diff --git a/tests/other/test_nested_user_callback.out b/tests/other/test_nested_user_callback.out new file mode 100644 index 0000000..988cea9 --- /dev/null +++ b/tests/other/test_nested_user_callback.out
@@ -0,0 +1,6 @@ +in main +nested call to callUserCallback detected (This should only be used to enter from the event loop) +in user callback +before: false +in user callback; without runtimeKeepalivePush +myatexit
diff --git a/tests/test_other.py b/tests/test_other.py index 1b236bd..0ed1ac7 100644 --- a/tests/test_other.py +++ b/tests/test_other.py
@@ -11299,11 +11299,16 @@ self.do_runf(test_file('hello_world.c'), 'hello, world!') def test_runtime_keepalive(self): - self.uses_es6 = True self.set_setting('DEFAULT_LIBRARY_FUNCS_TO_INCLUDE', ['$runtimeKeepalivePush', '$runtimeKeepalivePop', '$callUserCallback']) self.set_setting('EXIT_RUNTIME') self.do_other_test('test_runtime_keepalive.cpp') + def test_nested_user_callback(self): + self.set_setting('DEFAULT_LIBRARY_FUNCS_TO_INCLUDE', ['$runtimeKeepalivePush', '$runtimeKeepalivePop', '$callUserCallback']) + self.set_setting('EXIT_RUNTIME') + self.set_setting('ASSERTIONS') + self.do_other_test('test_nested_user_callback.c') + def test_em_asm_side_module(self): err = self.expect_fail([EMCC, '-sSIDE_MODULE', test_file('hello_world_em_asm.c')]) self.assertContained('EM_ASM is not supported in side modules', err)