Skip to content

Commit 751c5c0

Browse files
committed
fix(locker): correct Enter/Exit ordering in Locker
Enter() must be called after acquiring the lock, and Exit() before releasing it, because V8's entry_stack_ is not thread-safe. - new: Lock -> Enter (hold lock before touching entry_stack_) - drop: Exit -> Unlock (exit while still holding the lock)
1 parent 887fa4b commit 751c5c0

2 files changed

Lines changed: 15 additions & 31 deletions

File tree

src/isolate.rs

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2589,45 +2589,28 @@ pub struct Locker<'a> {
25892589
isolate: &'a mut UnenteredIsolate,
25902590
}
25912591

2592-
/// Guard to ensure `v8__Isolate__Exit` is called if panic occurs after Enter.
2593-
struct IsolateExitGuard(*mut RealIsolate);
2594-
2595-
impl Drop for IsolateExitGuard {
2596-
fn drop(&mut self) {
2597-
unsafe { v8__Isolate__Exit(self.0) };
2598-
}
2599-
}
2600-
26012592
impl<'a> Locker<'a> {
26022593
/// Creates a new `Locker` for the given isolate.
26032594
///
26042595
/// This will:
2605-
/// 1. Enter the isolate (via `v8::Isolate::Enter()`)
2606-
/// 2. Acquire the V8 lock (via `v8::Locker`)
2596+
/// 1. Acquire the V8 lock (via `v8::Locker`)
2597+
/// 2. Enter the isolate (via `v8::Isolate::Enter()`)
26072598
///
2608-
/// When the `Locker` is dropped, the lock is released and the isolate is exited.
2599+
/// When the `Locker` is dropped, the isolate is exited and the lock is released.
26092600
///
2610-
/// # Panics
2611-
///
2612-
/// This function is panic-safe. If initialization fails, the isolate will be
2613-
/// properly exited.
2601+
/// The ordering is critical: we must hold the lock before calling Enter(),
2602+
/// because Enter() modifies V8's entry_stack_ which is not thread-safe.
26142603
pub fn new(isolate: &'a mut UnenteredIsolate) -> Self {
26152604
let isolate_ptr = isolate.cxx_isolate;
26162605

2617-
// Enter the isolate first
2618-
unsafe {
2619-
v8__Isolate__Enter(isolate_ptr.as_ptr());
2620-
}
2621-
2622-
// Create exit guard - will call Exit if we panic before completing
2623-
let exit_guard = IsolateExitGuard(isolate_ptr.as_ptr());
2624-
2625-
// Initialize the raw Locker
2606+
// Acquire the lock first (must hold lock before touching entry_stack_)
26262607
let mut raw = unsafe { crate::scope::raw::Locker::uninit() };
26272608
unsafe { raw.init(isolate_ptr) };
26282609

2629-
// Success - forget the guard so it doesn't call Exit
2630-
std::mem::forget(exit_guard);
2610+
// Now enter the isolate (safe because we hold the lock)
2611+
unsafe {
2612+
v8__Isolate__Enter(isolate_ptr.as_ptr());
2613+
}
26312614

26322615
Self {
26332616
raw: std::mem::ManuallyDrop::new(raw),
@@ -2644,8 +2627,10 @@ impl<'a> Locker<'a> {
26442627
impl Drop for Locker<'_> {
26452628
fn drop(&mut self) {
26462629
unsafe {
2647-
std::mem::ManuallyDrop::drop(&mut self.raw);
2630+
// Exit first (while we still hold the lock), then release the lock.
2631+
// Reverse order of new(): Lock -> Enter, so drop: Exit -> Unlock.
26482632
v8__Isolate__Exit(self.isolate.cxx_isolate.as_ptr());
2633+
std::mem::ManuallyDrop::drop(&mut self.raw);
26492634
}
26502635
}
26512636
}

src/scope/raw.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,7 @@ impl Drop for AllowJavascriptExecutionScope {
236236
/// the `Locker` is dropped. Dropping an uninitialized `Locker` is undefined
237237
/// behavior because `Drop` will call the C++ destructor on garbage data.
238238
///
239-
/// 2. **Isolate State**: The isolate passed to `init()` must be in "entered" state
240-
/// (via `v8::Isolate::Enter()`) before calling `init()`.
239+
/// 2. **Isolate Pointer**: The isolate pointer passed to `init()` must be valid.
241240
///
242241
/// 3. **Single Initialization**: `init()` must be called exactly once. Calling it
243242
/// multiple times is undefined behavior.
@@ -278,7 +277,7 @@ impl Locker {
278277
/// # Safety
279278
///
280279
/// - This must be called exactly once after `uninit()`
281-
/// - The isolate must be valid and in "entered" state
280+
/// - The isolate pointer must be valid
282281
/// - The isolate must not be locked by another `Locker`
283282
/// - After this call, the `Locker` owns the V8 lock until dropped
284283
#[inline]

0 commit comments

Comments
 (0)