Skip to content

Commit 2760f83

Browse files
aheejinaheejin
authored
[embind] Manage exceptions' states correctly (#26519)
`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. #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 #26290.
1 parent 840a043 commit 2760f83

6 files changed

Lines changed: 157 additions & 9 deletions

File tree

‎src/lib/libemval.js‎

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
var LibraryEmVal = {
1212
// Stack of handles available for reuse.
1313
$emval_freelist: [],
14+
#if !DISABLE_EXCEPTION_CATCHING || WASM_EXCEPTIONS
15+
$emval_exception_decrefs: [],
16+
#endif
1417
// Array of alternating pairs (value, refcount).
1518
// reserve 0 and some special values. These never get de-allocated.
1619
$emval_handles: [
@@ -80,13 +83,27 @@ var LibraryEmVal = {
8083
}
8184
},
8285

83-
_emval_decref__deps: ['$emval_freelist', '$emval_handles'],
86+
_emval_decref__deps: ['$emval_freelist', '$emval_handles',
87+
#if !DISABLE_EXCEPTION_CATCHING || WASM_EXCEPTIONS
88+
'$emval_exception_decrefs',
89+
#endif
90+
],
8491
_emval_decref: (handle) => {
8592
if (handle > {{{ EMVAL_LAST_RESERVED_HANDLE }}} && 0 === --emval_handles[handle + 1]) {
8693
#if ASSERTIONS
8794
assert(emval_handles[handle] !== undefined, `Decref for unallocated handle.`);
8895
#endif
96+
var value = emval_handles[handle];
8997
emval_handles[handle] = undefined;
98+
#if !DISABLE_EXCEPTION_CATCHING || WASM_EXCEPTIONS
99+
// In case the value is a C++ exception, decrement the refcount, so the
100+
// memory can be freed correctly
101+
var destructor = emval_exception_decrefs[handle];
102+
if (destructor) {
103+
emval_exception_decrefs[handle] = undefined;
104+
destructor(value);
105+
}
106+
#endif
90107
emval_freelist.push(handle);
91108
}
92109
},
@@ -392,18 +409,40 @@ ${functionBody}
392409
return delete object[property];
393410
},
394411

412+
#if !DISABLE_EXCEPTION_CATCHING || WASM_EXCEPTIONS
413+
$isCppExceptionObject__deps: ['$Emval'],
414+
$isCppExceptionObject: (object) => {
415+
#if !DISABLE_EXCEPTION_CATCHING
416+
return object instanceof CppException;
417+
#else // WASM_EXCEPTIONS
418+
return object instanceof WebAssembly.Exception;
419+
#endif
420+
},
421+
#endif
422+
395423
_emval_throw__deps: ['$Emval',
396-
#if !WASM_EXCEPTIONS && !DISABLE_EXCEPTION_CATCHING
424+
#if !DISABLE_EXCEPTION_CATCHING || WASM_EXCEPTIONS
425+
#if !DISABLE_EXCEPTION_CATCHING
397426
'$exceptionLast',
427+
'$ExceptionInfo',
428+
#endif
429+
'$incrementExceptionRefcount',
430+
'$incrementUncaughtExceptionCount',
431+
'$isCppExceptionObject',
398432
#endif
399433
],
400434
_emval_throw: (object) => {
401435
object = Emval.toValue(object);
402-
#if !WASM_EXCEPTIONS && !DISABLE_EXCEPTION_CATCHING
403-
// If we are throwing Emcripten C++ exception, set exceptionLast, as we do
404-
// in __cxa_throw. C++ exception will be an instance of CppEmscripten.
405-
if (object instanceof CppException) {
436+
#if !DISABLE_EXCEPTION_CATCHING || WASM_EXCEPTIONS
437+
if (isCppExceptionObject(object)) {
438+
#if !DISABLE_EXCEPTION_CATCHING
439+
var info = new ExceptionInfo(object.excPtr);
440+
info.set_caught(false);
441+
info.set_rethrown(false);
406442
exceptionLast = object;
443+
#endif
444+
incrementUncaughtExceptionCount();
445+
incrementExceptionRefcount(object);
407446
}
408447
#endif
409448
throw object;
@@ -446,7 +485,15 @@ ${functionBody}
446485
}));
447486
},
448487

449-
_emval_from_current_cxa_exception__deps: ['$Emval', '__cxa_rethrow'],
488+
_emval_from_current_cxa_exception__deps: ['$Emval', '__cxa_rethrow',
489+
#if !DISABLE_EXCEPTION_THROWING || WASM_EXCEPTIONS
490+
'$decrementUncaughtExceptionCount',
491+
#endif
492+
#if !DISABLE_EXCEPTION_CATCHING || WASM_EXCEPTIONS
493+
'$decrementExceptionRefcount',
494+
'$emval_exception_decrefs',
495+
#endif
496+
],
450497
_emval_from_current_cxa_exception: () => {
451498
try {
452499
// Use __cxa_rethrow which already has mechanism for generating
@@ -455,7 +502,16 @@ ${functionBody}
455502
// with metadata optimised out otherwise.
456503
___cxa_rethrow();
457504
} catch (e) {
458-
return Emval.toHandle(e);
505+
#if !DISABLE_EXCEPTION_THROWING || WASM_EXCEPTIONS
506+
// ___cxa_rethrow incremented uncaughtExceptionCount.
507+
// Since we caught it in JS, we need to manually decrement it to balance.
508+
decrementUncaughtExceptionCount();
509+
#endif
510+
var handle = Emval.toHandle(e);
511+
#if !DISABLE_EXCEPTION_CATCHING || WASM_EXCEPTIONS
512+
emval_exception_decrefs[handle] = decrementExceptionRefcount;
513+
#endif
514+
return handle;
459515
}
460516
},
461517
};

‎src/lib/libexceptions.js‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,12 @@ var LibraryExceptions = {
366366
return ___thrown_object_from_unwind_exception(unwind_header);
367367
},
368368

369+
$incrementUncaughtExceptionCount__deps: ['__increment_uncaught_exception'],
370+
$incrementUncaughtExceptionCount: '__increment_uncaught_exception',
371+
372+
$decrementUncaughtExceptionCount__deps: ['__decrement_uncaught_exception'],
373+
$decrementUncaughtExceptionCount: '__decrement_uncaught_exception',
374+
369375
$incrementExceptionRefcount__deps: ['__cxa_increment_exception_refcount', '$getCppExceptionThrownObjectFromWebAssemblyException'],
370376
$incrementExceptionRefcount: (ex) => {
371377
var ptr = getCppExceptionThrownObjectFromWebAssemblyException(ex);
@@ -384,7 +390,20 @@ var LibraryExceptions = {
384390
return getExceptionMessageCommon(ptr);
385391
},
386392

387-
#elif !DISABLE_EXCEPTION_CATCHING
393+
#else
394+
#if !DISABLE_EXCEPTION_THROWING
395+
$incrementUncaughtExceptionCount__deps: ['$uncaughtExceptionCount'],
396+
$incrementUncaughtExceptionCount: () => {
397+
uncaughtExceptionCount++;
398+
},
399+
400+
$decrementUncaughtExceptionCount__deps: ['$uncaughtExceptionCount'],
401+
$decrementUncaughtExceptionCount: () => {
402+
uncaughtExceptionCount--;
403+
},
404+
#endif
405+
406+
#if !DISABLE_EXCEPTION_CATCHING
388407
$incrementExceptionRefcount__deps: ['__cxa_increment_exception_refcount'],
389408
$incrementExceptionRefcount: (exn) => ___cxa_increment_exception_refcount(exn.excPtr),
390409

@@ -394,6 +413,7 @@ var LibraryExceptions = {
394413
$getExceptionMessage__deps: ['$getExceptionMessageCommon'],
395414
$getExceptionMessage: (exn) => getExceptionMessageCommon(exn.excPtr),
396415

416+
#endif
397417
#endif
398418
};
399419

‎system/lib/libcxxabi/src/cxa_exception_js_utils.cpp‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,17 @@ char* __get_exception_terminate_message(void* thrown_object) {
123123
free(type);
124124
return result;
125125
}
126+
127+
#ifdef __WASM_EXCEPTIONS__
128+
void __increment_uncaught_exception() throw() {
129+
__cxa_get_globals()->uncaughtExceptions += 1;
130+
}
131+
132+
void __decrement_uncaught_exception() throw() {
133+
__cxa_get_globals()->uncaughtExceptions -= 1;
134+
}
135+
#endif
136+
126137
} // extern "C"
127138

128139
} // namespace __cxxabiv1

‎test/embind/test_embind_throw_val_uncaught_and_refcount.cpp‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#include <emscripten/val.h>
2+
#include <exception>
3+
#include <iostream>
4+
5+
using namespace emscripten;
6+
7+
int main() {
8+
val error = val::null();
9+
try {
10+
throw std::runtime_error("test exception");
11+
} catch (const std::exception& e) {
12+
// Capture the exception
13+
error = val::take_ownership(
14+
emscripten::internal::_emval_from_current_cxa_exception());
15+
}
16+
17+
std::cout << "Captured exception." << std::endl;
18+
19+
int uncaught_before = std::uncaught_exceptions();
20+
std::cout << "Uncaught before throw 1: " << uncaught_before << std::endl;
21+
22+
// First throw
23+
try {
24+
std::cout << "Throwing 1..." << std::endl;
25+
error.throw_();
26+
} catch (const std::exception& e) {
27+
std::cout << "Caught 1: " << e.what() << std::endl;
28+
int uncaught_during = std::uncaught_exceptions();
29+
std::cout << "Uncaught during catch 1: " << uncaught_during << std::endl;
30+
}
31+
32+
int uncaught_between = std::uncaught_exceptions();
33+
std::cout << "Uncaught between throws: " << uncaught_between << std::endl;
34+
35+
// Second throw - if refcount was messed up, this might fail/crash
36+
try {
37+
std::cout << "Throwing 2..." << std::endl;
38+
error.throw_();
39+
} catch (const std::exception& e) {
40+
std::cout << "Caught 2: " << e.what() << std::endl;
41+
int uncaught_during = std::uncaught_exceptions();
42+
std::cout << "Uncaught during catch 2: " << uncaught_during << std::endl;
43+
}
44+
45+
std::cout << "Done." << std::endl;
46+
return 0;
47+
}

‎test/embind/test_embind_throw_val_uncaught_and_refcount.out‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Captured exception.
2+
Uncaught before throw 1: 0
3+
Throwing 1...
4+
Caught 1: test exception
5+
Uncaught during catch 1: 0
6+
Uncaught between throws: 0
7+
Throwing 2...
8+
Caught 2: test exception
9+
Uncaught during catch 2: 0
10+
Done.

‎test/test_core.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7782,6 +7782,10 @@ def test_embind_wasm_workers(self):
77827782
def test_embind_throw_cpp_exception(self):
77837783
self.do_run_in_out_file_test('embind/test_embind_throw_cpp_exception.cpp', cflags=['-lembind', '-std=c++20'])
77847784

7785+
@with_all_eh_sjlj
7786+
def test_embind_throw_val_uncaught_and_refcount(self):
7787+
self.do_run_in_out_file_test('embind/test_embind_throw_val_uncaught_and_refcount.cpp', cflags=['-lembind', '-std=c++20'])
7788+
77857789
@parameterized({
77867790
'': ('DEFAULT', False),
77877791
'all': ('ALL', False),

0 commit comments

Comments
 (0)