fix(locker): add panic safety and improve documentation

- Add IsolateExitGuard to ensure v8::Isolate::Exit is called if panic
  occurs during Locker::new() after Enter has been called
- Add comprehensive documentation for Locker and UnenteredIsolate
  including thread safety guarantees and usage examples
- Add debug_assert to verify no Locker is held when dropping UnenteredIsolate
- Add UnenteredIsolate::as_raw() method for low-level access
- Expand test coverage:
  - locker_state_preserved_across_locks
  - locker_drop_releases_lock
  - unentered_isolate_as_raw
  - locker_send_isolate_between_threads (verifies Send trait works)

This improves safety guarantees for multi-threaded isolate usage.

Author: RainyPixel

src/isolate.rs

@@ -2126,7 +2126,37 @@ impl AsMut<Isolate> for Isolate {
   }
 }
 
-/// An isolate that must be accessed via `Locker`. Does not auto-enter.
+/// An isolate that must be accessed via [`Locker`].
+///
+/// Unlike [`OwnedIsolate`], this isolate does not automatically enter itself
+/// upon creation. Instead, you must use a [`Locker`] to access it:
+///
+/// ```ignore
+/// let mut isolate = v8::Isolate::new_unentered(Default::default());
+///
+/// // Access the isolate through a Locker
+/// {
+///     let mut locker = v8::Locker::new(&mut isolate);
+///     let scope = &mut v8::HandleScope::new(&mut *locker);
+///     // ... use scope ...
+/// }
+///
+/// // The locker is dropped, isolate can be used from another thread
+/// ```
+///
+/// # Thread Safety
+///
+/// `UnenteredIsolate` implements `Send`, meaning it can be transferred between
+/// threads. However, V8 isolates are not thread-safe by themselves. You must:
+///
+/// 1. Only access the isolate through a [`Locker`]
+/// 2. Never have multiple `Locker`s for the same isolate simultaneously
+///    (V8 will block if you try)
+///
+/// # Dropping
+///
+/// When dropped, the isolate will be properly disposed. The drop will panic
+/// if a [`Locker`] is currently held for this isolate.
 #[derive(Debug)]
 pub struct UnenteredIsolate {
   cxx_isolate: NonNull<RealIsolate>,
@@ -2138,10 +2168,28 @@ impl UnenteredIsolate {
       cxx_isolate: NonNull::new(cxx_isolate).unwrap(),
     }
   }
+
+  /// Returns the raw pointer to the underlying V8 isolate.
+  ///
+  /// # Safety
+  ///
+  /// The returned pointer is only valid while this `UnenteredIsolate` exists
+  /// and should only be used while a [`Locker`] is held.
+  #[inline]
+  pub fn as_raw(&self) -> *mut RealIsolate {
+    self.cxx_isolate.as_ptr()
+  }
 }
 
 impl Drop for UnenteredIsolate {
   fn drop(&mut self) {
+    // Safety check: ensure no Locker is held
+    debug_assert!(
+      !crate::scope::raw::Locker::is_locked(self.cxx_isolate),
+      "Cannot drop UnenteredIsolate while a Locker is held. \
+       Drop the Locker first."
+    );
+
     unsafe {
       let isolate = Isolate::from_raw_ref_mut(&mut self.cxx_isolate);
       let snapshot_creator =
@@ -2157,7 +2205,10 @@ impl Drop for UnenteredIsolate {
   }
 }
 
-// Thread safety ensured by Locker.
+// SAFETY: UnenteredIsolate can be sent between threads because:
+// 1. The underlying V8 isolate is not accessed directly - all access goes through Locker
+// 2. Locker ensures proper synchronization when accessing the isolate
+// 3. V8's Locker internally uses a mutex to prevent concurrent access
 unsafe impl Send for UnenteredIsolate {}
 
 /// Collection of V8 heap information.
@@ -2461,25 +2512,73 @@ impl AsRef<Isolate> for Isolate {
 }
 
 /// Locks an isolate and enters it for the current thread.
+///
+/// This is a RAII wrapper around V8's `v8::Locker`. It ensures that the isolate
+/// is properly locked before any V8 operations and unlocked when dropped.
+///
+/// # Thread Safety
+///
+/// `Locker` does not implement `Send` or `Sync`. Once created, it must be used
+/// only on the thread where it was created. The underlying `UnenteredIsolate`
+/// implements `Send`, allowing it to be transferred between threads, but a new
+/// `Locker` must be created on each thread that needs to access the isolate.
+///
+/// # Panic Safety
+///
+/// `Locker::new()` is panic-safe. If a panic occurs during construction,
+/// the isolate will be properly exited via a drop guard.
 pub struct Locker<'a> {
   raw: std::mem::ManuallyDrop<crate::scope::raw::Locker>,
   isolate: &'a mut UnenteredIsolate,
 }
 
+/// Guard to ensure `v8__Isolate__Exit` is called if panic occurs after Enter.
+struct IsolateExitGuard(*mut RealIsolate);
+
+impl Drop for IsolateExitGuard {
+  fn drop(&mut self) {
+    unsafe { v8__Isolate__Exit(self.0) };
+  }
+}
+
 impl<'a> Locker<'a> {
+  /// Creates a new `Locker` for the given isolate.
+  ///
+  /// This will:
+  /// 1. Enter the isolate (via `v8::Isolate::Enter()`)
+  /// 2. Acquire the V8 lock (via `v8::Locker`)
+  ///
+  /// When the `Locker` is dropped, the lock is released and the isolate is exited.
+  ///
+  /// # Panics
+  ///
+  /// This function is panic-safe. If initialization fails, the isolate will be
+  /// properly exited.
   pub fn new(isolate: &'a mut UnenteredIsolate) -> Self {
     let isolate_ptr = isolate.cxx_isolate;
+
+    // Enter the isolate first
     unsafe {
       v8__Isolate__Enter(isolate_ptr.as_ptr());
     }
+
+    // Create exit guard - will call Exit if we panic before completing
+    let exit_guard = IsolateExitGuard(isolate_ptr.as_ptr());
+
+    // Initialize the raw Locker
     let mut raw = unsafe { crate::scope::raw::Locker::uninit() };
     unsafe { raw.init(isolate_ptr) };
+
+    // Success - forget the guard so it doesn't call Exit
+    std::mem::forget(exit_guard);
+
     Self {
       raw: std::mem::ManuallyDrop::new(raw),
       isolate,
     }
   }
 
+  /// Returns `true` if the given isolate is currently locked by any `Locker`.
   pub fn is_locked(isolate: &UnenteredIsolate) -> bool {
     crate::scope::raw::Locker::is_locked(isolate.cxx_isolate)
   }

tests/test_locker.rs

@@ -1,4 +1,6 @@
 use std::pin::pin;
+use std::sync::mpsc;
+use std::thread;
 
 #[test]
 fn locker_basic() {
@@ -81,10 +83,147 @@ fn locker_is_locked() {
   assert!(!v8::Locker::is_locked(&isolate));
   {
     let _locker = v8::Locker::new(&mut isolate);
+    // Locker is now held - but we can't call is_locked because we have &mut isolate
+    // The lock check happens internally
   }
   assert!(!v8::Locker::is_locked(&isolate));
 }
 
+#[test]
+fn locker_state_preserved_across_locks() {
+  let _setup_guard = setup();
+  let mut isolate = v8::Isolate::new_unentered(Default::default());
+
+  // First lock: create a global and set a value
+  let global_template = {
+    let mut locker = v8::Locker::new(&mut isolate);
+    let scope = pin!(v8::HandleScope::new(&mut *locker));
+    let scope = &mut scope.init();
+    let context = v8::Context::new(scope, Default::default());
+    let scope = &mut v8::ContextScope::new(scope, context);
+
+    // Set a global variable
+    let code = v8::String::new(scope, "globalThis.testValue = 123").unwrap();
+    let script = v8::Script::compile(scope, code, None).unwrap();
+    script.run(scope).unwrap();
+
+    // Get the global object template for later verification
+    context.global(scope)
+  };
+  drop(global_template);
+
+  // Second lock: verify state is preserved (new context won't have the value)
+  {
+    let mut locker = v8::Locker::new(&mut isolate);
+    let scope = pin!(v8::HandleScope::new(&mut *locker));
+    let scope = &mut scope.init();
+    let context = v8::Context::new(scope, Default::default());
+    let scope = &mut v8::ContextScope::new(scope, context);
+
+    // New context - value should not exist
+    let code = v8::String::new(scope, "typeof globalThis.testValue").unwrap();
+    let script = v8::Script::compile(scope, code, None).unwrap();
+    let result = script.run(scope).unwrap();
+    let result_str = result.to_rust_string_lossy(scope);
+    assert_eq!(result_str, "undefined");
+  }
+}
+
+#[test]
+fn locker_drop_releases_lock() {
+  let _setup_guard = setup();
+  let mut isolate = v8::Isolate::new_unentered(Default::default());
+
+  // Create and immediately drop a locker
+  {
+    let locker = v8::Locker::new(&mut isolate);
+    drop(locker);
+  }
+
+  // Should be able to create another locker without blocking
+  {
+    let _locker = v8::Locker::new(&mut isolate);
+  }
+
+  // Isolate should be unlocked now
+  assert!(!v8::Locker::is_locked(&isolate));
+}
+
+#[test]
+fn unentered_isolate_as_raw() {
+  let _setup_guard = setup();
+  let isolate = v8::Isolate::new_unentered(Default::default());
+
+  // as_raw should return a valid pointer
+  let ptr = isolate.as_raw();
+  assert!(!ptr.is_null());
+}
+
+#[test]
+fn locker_send_isolate_between_threads() {
+  let _setup_guard = setup();
+
+  // Create isolate on main thread
+  let mut isolate = v8::Isolate::new_unentered(Default::default());
+
+  // Use on main thread first
+  {
+    let mut locker = v8::Locker::new(&mut isolate);
+    let scope = pin!(v8::HandleScope::new(&mut *locker));
+    let scope = &mut scope.init();
+    let context = v8::Context::new(scope, Default::default());
+    let scope = &mut v8::ContextScope::new(scope, context);
+
+    let code = v8::String::new(scope, "1 + 1").unwrap();
+    let script = v8::Script::compile(scope, code, None).unwrap();
+    let result = script.run(scope).unwrap();
+    assert_eq!(result.to_integer(scope).unwrap().value(), 2);
+  }
+
+  // Send to another thread
+  let (tx, rx) = mpsc::channel();
+
+  let handle = thread::spawn(move || {
+    // Use isolate on worker thread
+    let mut locker = v8::Locker::new(&mut isolate);
+    let scope = pin!(v8::HandleScope::new(&mut *locker));
+    let scope = &mut scope.init();
+    let context = v8::Context::new(scope, Default::default());
+    let scope = &mut v8::ContextScope::new(scope, context);
+
+    let code = v8::String::new(scope, "2 + 2").unwrap();
+    let script = v8::Script::compile(scope, code, None).unwrap();
+    let result = script.run(scope).unwrap();
+    let value = result.to_integer(scope).unwrap().value();
+
+    // Send result back
+    tx.send(value).unwrap();
+
+    // Return isolate ownership
+    drop(locker);
+    isolate
+  });
+
+  // Wait for result
+  let result = rx.recv().unwrap();
+  assert_eq!(result, 4);
+
+  // Get isolate back and use again on main thread
+  let mut isolate = handle.join().unwrap();
+  {
+    let mut locker = v8::Locker::new(&mut isolate);
+    let scope = pin!(v8::HandleScope::new(&mut *locker));
+    let scope = &mut scope.init();
+    let context = v8::Context::new(scope, Default::default());
+    let scope = &mut v8::ContextScope::new(scope, context);
+
+    let code = v8::String::new(scope, "3 + 3").unwrap();
+    let script = v8::Script::compile(scope, code, None).unwrap();
+    let result = script.run(scope).unwrap();
+    assert_eq!(result.to_integer(scope).unwrap().value(), 6);
+  }
+}
+
 fn setup() -> impl Drop {
   use std::sync::Once;
   static INIT: Once = Once::new();