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)
