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

Return error when exception encountered after main thread finishes #2169

Conversation

eloparco
Copy link
Contributor

@eloparco eloparco commented May 2, 2023

Currently, if a thread is spawned and raises an exception after the main thread has finished already, iwasm returns with success, instead of returning 1 (i.e. error).

Since wasm_runtime_get_wasi_exit_code waits for all threads to finished, the exception check has to performed again after all threads have finished.

Comment on lines +750 to +753
if (wasm_runtime_get_exception(wasm_module_inst)) {
/* got an exception in spawned thread */
ret = 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should also apply similar changes in file product-mini/platforms/windows/main.c

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, done

@eloparco eloparco force-pushed the eloparco/exception-after-main-thread-finishes branch from e531a9f to 17b6825 Compare May 3, 2023 07:56
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

@wenyongh wenyongh merged commit 71d43f3 into bytecodealliance:main May 5, 2023
515 checks passed
yamt added a commit to yamt/wasm-micro-runtime that referenced this pull request Aug 31, 2023
…shes (bytecodealliance#2169)"

This reverts commit 71d43f3.

for some reasons, the original commit assumes a trap should win over exit
even if the former is 1 second later. i don't see any reason it should.
yamt added a commit to yamt/wasm-micro-runtime that referenced this pull request Aug 31, 2023
…shes (bytecodealliance#2169)"

This reverts commit 71d43f3.

for some reasons, the original commit assumes a trap should win over
an exit even if the former is 1 second behind the latter. i don't
see any reason it should. in that case, the expected behavior is that
the exit likely wins and the other thread won't even reach the code to
raise a trap.
yamt added a commit to yamt/wasm-micro-runtime that referenced this pull request Aug 31, 2023
…shes (bytecodealliance#2169)"

This reverts commit 71d43f3.

for some reasons, the original commit assumes a trap should win over
an exit even if the former is 1 second behind the latter. i don't
see any reason it should. in that case, the expected behavior is that
the exit likely wins and the other thread won't even reach the code to
raise a trap.
yamt added a commit to yamt/wasm-micro-runtime that referenced this pull request Sep 4, 2023
…shes (bytecodealliance#2169)"

This reverts commit 71d43f3.

for some reasons, the original commit assumes a trap should win over
an exit even if the former is 1 second behind the latter. i don't
see any reason it should. in that case, the expected behavior is that
the exit likely wins and the other thread won't even reach the code to
raise a trap.
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
…tecodealliance#2169)

Currently, if a thread is spawned and raises an exception after the main thread
has finished, iwasm returns with success instead of returning 1 (i.e. error).

Since wasm_runtime_get_wasi_exit_code waits for all threads to finish and only
returns the wasi exit code, this PR performs the exception check again and
returns error if an exception was raised.
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

2 participants