[embind] Manage exceptions' states correctly#26519
Open
aheejin wants to merge 5 commits intoemscripten-core:mainfrom
Open
[embind] Manage exceptions' states correctly#26519aheejin wants to merge 5 commits intoemscripten-core:mainfrom
aheejin wants to merge 5 commits intoemscripten-core:mainfrom
Conversation
`libemval.js` has two exception-related functions: `_emval_throw` and `_emval_from_current_cxa_exception`, but they haven't been correclty updating various exception-related states. emscripten-core#26276 tried to fix this but it only updated `exceptionLast`. This updates other internal states correctly. Some of them only apply to Emscripten EH, while others apply to both. 1. To make the uncaught exception count updatable for Wasm EH, this adds `___cxa_in/decrement_uncaught_exception` to `cxa_exception_js_utils.cpp`, and also adds a common interface `in/decrementUncaughtExceptionCount` in `libexceptions.js` so that you can call them regardless of which EH is used. For Wasm EH, the management of the uncaught exception count is done within libc++abi functions (`__cxa_throw`, `__cxa_rethrow`, ...), but here in embind we bypass those functions, so we have to manage them directly. 2. To make refcount work correctly, this adds calls to `in/decrementExceptionRefcount` to exception-related functions. Also, this fixes memory leaks by adding the destructor mechanism that decrements refcount when emvals are destroyed. `_emval_from_current_cxa_exception` is similar to `std::current_exception`, which returns `std::exception_ptr`, whose destructor decrements the refcount so that the exception can be destroyed: https://github.com/emscripten-core/emscripten/blob/62e22652509fbe7a00609ce48a653d0d66f27ba5/system/lib/libcxx/src/support/runtime/exception_pointer_cxxabi.ipp#L16 embind didn't have such a mechanism, so exceptions captured by `_emval_from_current_cxa_exception` leaked memory. This also fixes emscripten-core#26290.
This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (2) test expectation files were updated by running the tests with `--rebaseline`: ``` codesize/test_minimal_runtime_code_size_hello_embind.json: 14909 => 14961 [+52 bytes / +0.35%] codesize/test_minimal_runtime_code_size_hello_embind_val.json: 11642 => 11695 [+53 bytes / +0.46%] Average change: +0.40% (+0.35% - +0.46%) ```
sbc100
reviewed
Mar 23, 2026
| #if EXCEPTION_STACK_TRACES | ||
| return object instanceof CppException; | ||
| #else | ||
| return object === object+0; // Check if it is a number |
Collaborator
There was a problem hiding this comment.
Give that this seems to also return try for numbers maybe it should be emval_could_be_cpp_exception?
| if (destructor) { | ||
| emval_destructors[handle] = undefined; | ||
| destructor(value); | ||
| } |
Collaborator
There was a problem hiding this comment.
Wrap this new code in #if !DISABLE_EXCEPTION_THROWING || WASM_EXCEPTIONS?
| var LibraryEmVal = { | ||
| // Stack of handles available for reuse. | ||
| $emval_freelist: [], | ||
| $emval_destructors: [], |
Collaborator
There was a problem hiding this comment.
Online define if #if !DISABLE_EXCEPTION_THROWING || WASM_EXCEPTIONS.
Also, it might be misleading to simply call this emval_destructors. How about making it less generic and more sepcfic. e.g. emval_is_exception just stores booleans and then decrementExceptionRefcount is called explicitly during _emval_decref.
I fear that otherwise this looks too much like how embind handles cleanup / destructors for normal C++ objects (which it is not).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
libemval.jshas two exception-related functions:_emval_throwand_emval_from_current_cxa_exception, but they haven't been correclty updating various exception-related states. #26276 tried to fix this but it only updatedexceptionLast.This updates other internal states correctly. Some of them only apply to Emscripten EH, while others apply to both.
To make the uncaught exception count updatable for Wasm EH, this adds
___cxa_in/decrement_uncaught_exceptiontocxa_exception_js_utils.cpp, and also adds a common interfacein/decrementUncaughtExceptionCountinlibexceptions.jsso that you can call them regardless of which EH is used. For Wasm EH, the management of the uncaught exception count is done within libc++abi functions (__cxa_throw,__cxa_rethrow, ...), but here in embind we bypass those functions, so we have to manage them directly.To make refcount work correctly, this adds calls to
in/decrementExceptionRefcountto exception-related functions. Also, this fixes memory leaks by adding the destructor mechanism that decrements refcount when emvals are destroyed._emval_from_current_cxa_exceptionis similar tostd::current_exception, which returnsstd::exception_ptr, whose destructor decrements the refcount so that the exception can be destroyed:emscripten/system/lib/libcxx/src/support/runtime/exception_pointer_cxxabi.ipp
Line 16 in 62e2265
_emval_from_current_cxa_exceptionleaked memory.This also fixes #26290.