Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix keepRuntimeAlive in the case where EXIT_RUNTIME=0 and noExitRuntime is not referenced #22542

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -2142,10 +2142,24 @@ addToLibrary({
$runtimeKeepaliveCounter__internal: true,
$runtimeKeepaliveCounter: 0,

$keepRuntimeAlive__deps: ['$runtimeKeepaliveCounter'],
#if isSymbolNeeded('$noExitRuntime')
// If the `noExitRuntime` symbol is included in the build then
// keepRuntimeAlive is always conditional since its state can change
// at runtime.
$keepRuntimeAlive__deps: ['$runtimeKeepaliveCounter'],
$keepRuntimeAlive: () => noExitRuntime || runtimeKeepaliveCounter > 0,
#elif !EXIT_RUNTIME
// When `noExitRuntime` is not include and EXIT_RUNTIME=0 then we know the
// runtime can never exit (i.e. should always be kept alive).
// However for pthreads we always default to allowing the runtime to exit
// otherwise threads never exit and are not joinable.
#if PTHREADS
$keepRuntimeAlive: () => !ENVIRONMENT_IS_PTHREAD,
sbc100 marked this conversation as resolved.
Show resolved Hide resolved
#else
$keepRuntimeAlive: () => true,
#endif
#else
$keepRuntimeAlive__deps: ['$runtimeKeepaliveCounter'],
$keepRuntimeAlive: () => runtimeKeepaliveCounter > 0,
#endif

Expand Down
22 changes: 22 additions & 0 deletions test/other/test_no_exit_runtime_strict.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#include <assert.h>
#include <emscripten/emscripten.h>
#include <emscripten/em_js.h>
#include <stdio.h>

// Verify that `keepRuntimeAlive()` always returns true by default (i.e. when
// EXIT_RUNTIME=0).

EM_JS(void, timeout_func, (), {
console.log("timeout_func: keepRuntimeAlive() ->", keepRuntimeAlive());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you not need to ensure that keepRuntimeAlive is available in JS?

// once this timeout is done the node process should exit with 0
});

int main() {
int keep_alive = EM_ASM_INT({
setTimeout(timeout_func);
return keepRuntimeAlive();
}, emscripten_force_exit);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is a parameter passed in here?

printf("main done: %d\n", keep_alive);
assert(keep_alive == 1);
return 0;
}
2 changes: 2 additions & 0 deletions test/other/test_no_exit_runtime_strict.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
main done: 1
timeout_func: keepRuntimeAlive() -> true
3 changes: 3 additions & 0 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -5038,6 +5038,9 @@ def test(cxx, no_exit, assertions, flush=0, keepalive=0, filesystem=1):
for flush in (0, 1):
test(cxx, no_exit, assertions, flush)

def test_no_exit_runtime_strict(self):
self.do_other_test('test_no_exit_runtime_strict.c', emcc_args=['-sSTRICT'])

def test_extra_opt_levels(self):
# Opt levels that we don't tend to test elsewhere
for opt in ('-Og', '-Ofast'):
Expand Down
6 changes: 3 additions & 3 deletions tools/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,9 +658,6 @@ def phase_linker_setup(options, state, newargs):
# Add `#!` line to output JS and make it executable.
options.executable = True

if 'noExitRuntime' in settings.INCOMING_MODULE_JS_API:
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE.append('$noExitRuntime')

if settings.OPT_LEVEL >= 1:
default_setting('ASSERTIONS', 0)

Expand Down Expand Up @@ -914,6 +911,9 @@ def phase_linker_setup(options, state, newargs):
else:
default_setting('INCOMING_MODULE_JS_API', [])

if 'noExitRuntime' in settings.INCOMING_MODULE_JS_API:
settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE.append('$noExitRuntime')

if not settings.MINIMAL_RUNTIME and not settings.STRICT:
# Export the HEAP object by default, when not running in STRICT mode
settings.EXPORTED_RUNTIME_METHODS.extend([
Expand Down
Loading