Skip to content

Avoid calling process.exit under node - #18754

Merged
sbc100 merged 1 commit into
mainfrom
logExceptionOnExit_node
Feb 18, 2023
Merged

Avoid calling process.exit under node#18754
sbc100 merged 1 commit into
mainfrom
logExceptionOnExit_node

Conversation

@sbc100

@sbc100 sbc100 commented Feb 15, 2023

Copy link
Copy Markdown
Collaborator

Instead we always prefer to let exceptions bubble out and get handled by whatever handlers are installed.

Doing this did expose a few places where we were calling back into the runtime after it had exited. This was previously being masked by the fact that we were bringing down the entire node process. This means that when running tests under node we will now be a little more sensitive to use-after-exit bugs in our runtime and test code, but I would argue this is good thing.

This also means we no longer need to define logExceptionOnExit under node, which is a code size saving.

@sbc100
sbc100 requested a review from tlively February 15, 2023 02:45
@sbc100
sbc100 force-pushed the logExceptionOnExit_node branch from 383d41e to a7afed9 Compare February 15, 2023 06:22
Comment thread emscripten.py Outdated
@sbc100
sbc100 force-pushed the logExceptionOnExit_node branch 3 times, most recently from 69ff9f1 to a587987 Compare February 15, 2023 18:16
sbc100 added a commit that referenced this pull request Feb 15, 2023
This could help with some of the flaky thread tests we have been
seeing as it means that we never try to process messages from
already-terminated threads.

Split out from #18754
sbc100 added a commit that referenced this pull request Feb 15, 2023
This could help with some of the flaky thread tests we have been
seeing as it means that we never try to process messages from
already-terminated threads.

Split out from #18754
sbc100 added a commit that referenced this pull request Feb 15, 2023
This could help with some of the flaky thread tests we have been
seeing as it means that we never try to process messages from
already-terminated threads.

Split out from #18754
sbc100 added a commit that referenced this pull request Feb 15, 2023
This could help with some of the flaky thread tests we have been
seeing as it means that we never try to process messages from
already-terminated threads.

Split out from #18754
sbc100 added a commit that referenced this pull request Feb 15, 2023
This could help with some of the flaky thread tests we have been
seeing as it means that we never try to process messages from
already-terminated threads.

Split out from #18754
sbc100 added a commit that referenced this pull request Feb 16, 2023
This could help with some of the flaky thread tests we have been
seeing as it means that we never try to process messages from
already-terminated threads.

Split out from #18754
sbc100 added a commit that referenced this pull request Feb 16, 2023
This could help with some of the flaky thread tests we have been
seeing as it means that we never try to process messages from
already-terminated threads.

Split out from #18754
@sbc100
sbc100 force-pushed the logExceptionOnExit_node branch 10 times, most recently from 054d157 to b1f1efe Compare February 17, 2023 22:47
Instead we always prefer to let exceptions bubble out and get handles by
whatever handlers are installed.

Doing this did exposed a few places where we were calling back into
the runtime after it had exited.  This was previously being masked by
the fact that we were bringing down the entire node process.  This means
that when running tests under node we will now be a little more
sensitive to use-after-exit bugs in our runtime and test code, but I
would argue this is good thing.

This also means we no longer need to define logExceptionOnExit under
node, which is node code size saving.
@sbc100
sbc100 force-pushed the logExceptionOnExit_node branch from b1f1efe to a6d108a Compare February 18, 2023 21:04
@sbc100
sbc100 enabled auto-merge (squash) February 18, 2023 21:14
@sbc100
sbc100 merged commit fa339b7 into main Feb 18, 2023
@sbc100
sbc100 deleted the logExceptionOnExit_node branch February 18, 2023 21:15
sbc100 added a commit that referenced this pull request Feb 23, 2023
This was fixed in #18754. Adding a test to confirm it.

See: #15080
sbc100 added a commit that referenced this pull request Feb 23, 2023
This was fixed in #18754. Adding a test to confirm it.

See: #15080
sbc100 added a commit that referenced this pull request Feb 23, 2023
This was fixed in #18754. Adding a test to confirm it.

See: #15080
sbc100 added a commit that referenced this pull request Feb 23, 2023
This was fixed in #18754. Adding a test to confirm it.

See: #15080
sbc100 added a commit that referenced this pull request Feb 23, 2023
This was fixed in #18754. Adding a test to confirm it.

See: #15080
sbc100 added a commit to sbc100/emscripten that referenced this pull request Aug 27, 2025
The `checkMailbox` callback can occur after the thread has terminated.
In this case calling into native code can trigger the `makeAbortWrapper`
wrapper that is put around each native function which then results in a
"program has already aborted!" error being thrown.

Once solution to this is to make sure that the function which are called
do not have `makeAbortWrapper` applied to them.

This was the technique I used in emscripten-core#18754, but the list of functions
became stale when emscripten_proxy_execute_task_queue was removed in emscripten-core#18852.

A better solution is to wrap to whole function in callUserCallback,
which takes case of checking if the runtime is alive before calling into
native code.

Fixes: emscripten-core#20067
sbc100 added a commit to sbc100/emscripten that referenced this pull request Aug 27, 2025
The `checkMailbox` callback can occur after the thread has terminated.
In this case calling into native code can trigger the `makeAbortWrapper`
wrapper that is put around each native function which then results in a
"program has already aborted!" error being thrown.

Once solution to this is to make sure that the function which are called
do not have `makeAbortWrapper` applied to them.

This was the technique I used in emscripten-core#18754, but the list of functions
became stale when emscripten_proxy_execute_task_queue was removed in emscripten-core#18852.

A better solution is to wrap to whole function in callUserCallback,
which takes case of checking if the runtime is alive before calling into
native code.

Fixes: emscripten-core#20067
sbc100 added a commit to sbc100/emscripten that referenced this pull request Aug 27, 2025
The `checkMailbox` callback can occur after the thread has terminated.
In this case calling into native code can trigger the `makeAbortWrapper`
wrapper that is put around each native function which then results in a
"program has already aborted!" error being thrown.

Once solution to this is to make sure that the function which are called
do not have `makeAbortWrapper` applied to them.

This was the technique I used in emscripten-core#18754, but the list of functions
became stale when emscripten_proxy_execute_task_queue was removed in emscripten-core#18852.

A better solution is to wrap to whole function in callUserCallback,
which takes case of checking if the runtime is alive before calling into
native code.

Fixes: emscripten-core#20067
sbc100 added a commit to sbc100/emscripten that referenced this pull request Aug 27, 2025
The `checkMailbox` callback can occur after the thread has terminated.
In this case calling into native code can trigger the `makeAbortWrapper`
wrapper that is put around each native function which then results in a
"program has already aborted!" error being thrown.

Once solution to this is to make sure that the function which are called
do not have `makeAbortWrapper` applied to them.

This was the technique I used in emscripten-core#18754, but the list of functions
became stale when emscripten_proxy_execute_task_queue was removed in emscripten-core#18852.

A better solution is to wrap to whole function in callUserCallback,
which takes case of checking if the runtime is alive before calling into
native code.

Fixes: emscripten-core#20067
sbc100 added a commit to sbc100/emscripten that referenced this pull request Aug 27, 2025
The `checkMailbox` callback can occur after the thread has terminated.
In this case calling into native code can trigger the `makeAbortWrapper`
wrapper that is put around each native function which then results in a
"program has already aborted!" error being thrown.

Once solution to this is to make sure that the function which are called
do not have `makeAbortWrapper` applied to them.

This was the technique I used in emscripten-core#18754, but the list of functions
became stale when emscripten_proxy_execute_task_queue was removed in emscripten-core#18852.

A better solution is to wrap to whole function in callUserCallback,
which takes case of checking if the runtime is alive before calling into
native code.

Fixes: emscripten-core#20067
sbc100 added a commit that referenced this pull request Aug 28, 2025
The `checkMailbox` callback can occur after the thread has terminated.
In this case calling into native code can trigger the `makeAbortWrapper`
wrapper that is put around each native function which then results in a
"program has already aborted!" error being thrown.

Once solution to this is to make sure that the function which are called
do not have `makeAbortWrapper` applied to them.

This was the technique I used in #18754, but the list of functions
became stale when emscripten_proxy_execute_task_queue was removed in
#18852.

A better solution is to wrap to whole function in callUserCallback,
which takes care of checking if the runtime is alive before calling into
native code.

Fixes: #20067
inolen pushed a commit to inolen/emscripten that referenced this pull request Feb 13, 2026
The `checkMailbox` callback can occur after the thread has terminated.
In this case calling into native code can trigger the `makeAbortWrapper`
wrapper that is put around each native function which then results in a
"program has already aborted!" error being thrown.

Once solution to this is to make sure that the function which are called
do not have `makeAbortWrapper` applied to them.

This was the technique I used in emscripten-core#18754, but the list of functions
became stale when emscripten_proxy_execute_task_queue was removed in
emscripten-core#18852.

A better solution is to wrap to whole function in callUserCallback,
which takes care of checking if the runtime is alive before calling into
native code.

Fixes: emscripten-core#20067
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants