0
0
Fork 0
mirror of https://github.com/denoland/rusty_v8.git synced 2024-12-25 16:49:29 -05:00

Explicit panic instead of silent memory corruption (#1377)

Due to the automatic entry and exit behavior of Isolate upon creation and drop, it is crucial to ensure that v8::OwnedIsolate instances are dropped in the reverse order of their creation. Dropping them in the incorrect order can result in the corruption of the thread-local stack managed by v8, leading to memory corruption and potential segfaults. This introduces a check to verify the `this == Isolate::GetCurrent()` requirement before invoking the exit function. If the requirement is not met, a clean panic is triggered to provide explicit error handling instead of allowing silent memory corruption.
This commit is contained in:
Guillaume Bort 2023-12-12 15:32:40 +01:00 committed by GitHub
parent 811cce27c0
commit 60e0859514
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 21 additions and 1 deletions

View file

@ -161,6 +161,8 @@ void v8__Isolate__Enter(v8::Isolate* isolate) { isolate->Enter(); }
void v8__Isolate__Exit(v8::Isolate* isolate) { isolate->Exit(); }
v8::Isolate* v8__Isolate__GetCurrent() { return v8::Isolate::GetCurrent(); }
void v8__Isolate__MemoryPressureNotification(v8::Isolate* isolate,
v8::MemoryPressureLevel level) {
isolate->MemoryPressureNotification(level);

View file

@ -379,6 +379,7 @@ extern "C" {
fn v8__Isolate__GetNumberOfDataSlots(this: *const Isolate) -> u32;
fn v8__Isolate__Enter(this: *mut Isolate);
fn v8__Isolate__Exit(this: *mut Isolate);
fn v8__Isolate__GetCurrent() -> *mut Isolate;
fn v8__Isolate__MemoryPressureNotification(this: *mut Isolate, level: u8);
fn v8__Isolate__ClearKeptObjects(isolate: *mut Isolate);
fn v8__Isolate__LowMemoryNotification(isolate: *mut Isolate);
@ -534,7 +535,8 @@ extern "C" {
/// synchronize.
///
/// rusty_v8 note: Unlike in the C++ API, the Isolate is entered when it is
/// constructed and exited when dropped.
/// constructed and exited when dropped. Because of that v8::OwnedIsolate
/// instances must be dropped in the reverse order of creation
#[repr(C)]
#[derive(Debug)]
pub struct Isolate(Opaque);
@ -1518,6 +1520,11 @@ impl Drop for OwnedIsolate {
snapshot_creator.is_none(),
"If isolate was created using v8::Isolate::snapshot_creator, you should use v8::OwnedIsolate::create_blob before dropping an isolate."
);
// Safety: We need to check `this == Isolate::GetCurrent()` before calling exit()
assert!(
self.cxx_isolate.as_mut() as *mut Isolate == v8__Isolate__GetCurrent(),
"v8::OwnedIsolate instances must be dropped in the reverse order of creation. They are entered upon creation and exited upon being dropped."
);
self.exit();
self.cxx_isolate.as_mut().clear_scope_and_annex();
self.cxx_isolate.as_mut().dispose();

View file

@ -636,6 +636,17 @@ fn microtasks() {
}
}
#[test]
#[should_panic(
expected = "v8::OwnedIsolate instances must be dropped in the reverse order of creation. They are entered upon creation and exited upon being dropped."
)]
fn isolate_drop_order() {
let isolate1 = v8::Isolate::new(Default::default());
let isolate2 = v8::Isolate::new(Default::default());
drop(isolate1);
drop(isolate2);
}
#[test]
fn get_isolate_from_handle() {
extern "C" {