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

set_exception_visitor: remove the special case for wasi proc exit #2525

Merged
merged 1 commit into from
Sep 4, 2023

Conversation

yamt
Copy link
Collaborator

@yamt yamt commented Aug 31, 2023

while wasi proc exit is not a real trap, what the runtime does on it is mostly same as real traps. that is, kill the siblings threads and represent the exit/trap as the result of the "process" to the user api. there seems no reason to distinuguish it from real traps here.

note that:

  • the target thread either doesn't care the specific exception type or ignore wasi proc exit by themselves. (clear_wasi_proc_exit_exception)

  • clear_wasi_proc_exit_exception only clears local exception.

while wasi proc exit is not a real trap, what the runtime does on it
is mostly same as real traps. that is, kill the siblings threads
and represent the exit/trap as the result of the "process" to
the user api. there seems no reason to distinuguish it from
real traps here.

note that:

* the target thread either doesn't care the specific exception type
  or ignore wasi proc exit by themselves. (clear_wasi_proc_exit_exception)

* clear_wasi_proc_exit_exception only clears local exception.
Copy link
Contributor

@wenyongh wenyongh left a comment

Choose a reason for hiding this comment

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

LGTM

@eloparco, could you help review this PR? Seems the issue mentioned in #1988 was already resolved since function clear_wasi_proc_exit_exception only clears the exception of current thread, so we can spread "wasi proc exit" exception to other threads to let them exit ASAP (the terminate flag is also set to other threads).

@eloparco
Copy link
Contributor

eloparco commented Sep 4, 2023

Things have changed since that PR and it seems that the distinction is not needed anymore. We have multiple tests that check exception propagation and the CI is green, so we should be confident to merge.

@wenyongh wenyongh merged commit 2e78230 into bytecodealliance:main Sep 4, 2023
368 checks passed
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
…tecodealliance#2525)

While wasi proc exit is not a real trap, what the runtime does on it is mostly same as
real traps. That is, kill the siblings threads and represent the exit/trap as the result of
the "process" to the user api. There seems no reason to distinguish it from real traps
here.

Note that:
- The target thread either doesn't care the specific exception type or ignore wasi
  proc exit by themselves. (clear_wasi_proc_exit_exception)
- clear_wasi_proc_exit_exception only clears local exception.
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.

None yet

3 participants