Use correct lifetime for TryCatch::exception()/message() return value

src/try_catch.rs

@@ -110,9 +110,28 @@ impl<'tc> TryCatch<'tc> {
   /// Returns the exception caught by this try/catch block. If no exception has
   /// been caught an empty handle is returned.
   ///
-  /// The returned handle is valid until this TryCatch block has been destroyed.
-  pub fn exception(&self) -> Option<Local<'tc, Value>> {
-    unsafe { Local::from_raw(v8__TryCatch__Exception(&self.0)) }
+  /// Note: v8.h states that "the returned handle is valid until this TryCatch
+  /// block has been destroyed". This is incorrect; the return value lives
+  /// no longer and no shorter than the active HandleScope at the time this
+  /// method is called. An issue has been opened about this in the V8 bug
+  /// tracker: https://bugs.chromium.org/p/v8/issues/detail?id=10537.
+  pub fn exception<'sc>(
+    &self,
+    scope: &mut impl ToLocal<'sc>,
+  ) -> Option<Local<'sc, Value>> {
+    unsafe { scope.to_local(v8__TryCatch__Exception(&self.0)) }
+  }
+
+  /// Returns the message associated with this exception. If there is
+  /// no message associated an empty handle is returned.
+  ///
+  /// Note: the remark about the lifetime for the `exception()` return value
+  /// applies here too.
+  pub fn message<'sc>(
+    &self,
+    scope: &mut impl ToLocal<'sc>,
+  ) -> Option<Local<'sc, Message>> {
+    unsafe { scope.to_local(v8__TryCatch__Message(&self.0)) }
   }
 
   /// Returns the .stack property of the thrown object. If no .stack
@@ -125,15 +144,6 @@ impl<'tc> TryCatch<'tc> {
     unsafe { scope.to_local(v8__TryCatch__StackTrace(&self.0, &*context)) }
   }
 
-  /// Returns the message associated with this exception. If there is
-  /// no message associated an empty handle is returned.
-  ///
-  /// The returned handle is valid until this TryCatch block has been
-  /// destroyed.
-  pub fn message(&self) -> Option<Local<'tc, Message>> {
-    unsafe { Local::from_raw(v8__TryCatch__Message(&self.0)) }
-  }
-
   /// Clears any exceptions that may have been caught by this try/catch block.
   /// After this method has been called, HasCaught() will return false. Cancels
   /// the scheduled exception if it is caught and ReThrow() is not called before.

tests/compile_fail/try_catch_exception_lifetime.rs

@@ -3,13 +3,13 @@ use rusty_v8 as v8;
 
 pub fn main() {
   let mut isolate = v8::Isolate::new(mock());
-  let mut hs = v8::HandleScope::new(&mut isolate);
-  let hs = hs.enter();
+  let mut try_catch = v8::TryCatch::new(&mut isolate);
+  let try_catch = try_catch.enter();
 
   let _exception = {
-    let mut try_catch = v8::TryCatch::new(hs);
-    let tc = try_catch.enter();
-    tc.exception().unwrap()
+    let mut hs = v8::HandleScope::new(&mut isolate);
+    let hs = hs.enter();
+    try_catch.exception(hs).unwrap()
   };
 }
 

tests/compile_fail/try_catch_exception_lifetime.stderr

@@ -1,11 +1,11 @@
-error[E0597]: `try_catch` does not live long enough
+error[E0597]: `hs` does not live long enough
   --> $DIR/try_catch_exception_lifetime.rs:11:14
    |
 9  |   let _exception = {
    |       ---------- borrow later stored here
-10 |     let mut try_catch = v8::TryCatch::new(hs);
-11 |     let tc = try_catch.enter();
-   |              ^^^^^^^^^ borrowed value does not live long enough
-12 |     tc.exception().unwrap()
+10 |     let mut hs = v8::HandleScope::new(&mut isolate);
+11 |     let hs = hs.enter();
+   |              ^^ borrowed value does not live long enough
+12 |     try_catch.exception(hs).unwrap()
 13 |   };
-   |   - `try_catch` dropped here while still borrowed
+   |   - `hs` dropped here while still borrowed

tests/compile_fail/try_catch_message_lifetime.rs

@@ -3,13 +3,13 @@ use rusty_v8 as v8;
 
 pub fn main() {
   let mut isolate = v8::Isolate::new(mock());
-  let mut hs = v8::HandleScope::new(&mut isolate);
-  let hs = hs.enter();
+  let mut try_catch = v8::TryCatch::new(&mut isolate);
+  let try_catch = try_catch.enter();
 
-  let _message = {
-    let mut try_catch = v8::TryCatch::new(hs);
-    let tc = try_catch.enter();
-    tc.message().unwrap()
+  let _exception = {
+    let mut hs = v8::HandleScope::new(&mut isolate);
+    let hs = hs.enter();
+    try_catch.message(hs).unwrap()
   };
 }
 

tests/compile_fail/try_catch_message_lifetime.stderr

@@ -1,11 +1,11 @@
-error[E0597]: `try_catch` does not live long enough
+error[E0597]: `hs` does not live long enough
   --> $DIR/try_catch_message_lifetime.rs:11:14
    |
-9  |   let _message = {
-   |       -------- borrow later stored here
-10 |     let mut try_catch = v8::TryCatch::new(hs);
-11 |     let tc = try_catch.enter();
-   |              ^^^^^^^^^ borrowed value does not live long enough
-12 |     tc.message().unwrap()
+9  |   let _exception = {
+   |       ---------- borrow later stored here
+10 |     let mut hs = v8::HandleScope::new(&mut isolate);
+11 |     let hs = hs.enter();
+   |              ^^ borrowed value does not live long enough
+12 |     try_catch.message(hs).unwrap()
 13 |   };
-   |   - `try_catch` dropped here while still borrowed
+   |   - `hs` dropped here while still borrowed

tests/test_api.rs

@@ -536,11 +536,14 @@ fn try_catch() {
       let result = eval(scope, context, "throw new Error('foo')");
       assert!(result.is_none());
       assert!(tc.has_caught());
-      assert!(tc.exception().is_some());
+      assert!(tc.exception(scope).is_some());
       assert!(tc.stack_trace(scope, context).is_some());
-      assert!(tc.message().is_some());
+      assert!(tc.message(scope).is_some());
       assert_eq!(
-        tc.message().unwrap().get(scope).to_rust_string_lossy(scope),
+        tc.message(scope)
+          .unwrap()
+          .get(scope)
+          .to_rust_string_lossy(scope),
         "Uncaught Error: foo"
       );
     };
@@ -551,9 +554,9 @@ fn try_catch() {
       let result = eval(scope, context, "1 + 1");
       assert!(result.is_some());
       assert!(!tc.has_caught());
-      assert!(tc.exception().is_none());
+      assert!(tc.exception(scope).is_none());
       assert!(tc.stack_trace(scope, context).is_none());
-      assert!(tc.message().is_none());
+      assert!(tc.message(scope).is_none());
       assert!(tc.rethrow().is_none());
     };
     {
@@ -574,6 +577,41 @@ fn try_catch() {
   }
 }
 
+#[test]
+fn try_catch_caught_lifetime() {
+  let _setup_guard = setup();
+  let mut isolate = v8::Isolate::new(Default::default());
+  let mut hs = v8::HandleScope::new(&mut isolate);
+  let scope = hs.enter();
+  let context = v8::Context::new(scope);
+  let mut cs = v8::ContextScope::new(scope, context);
+  let scope = cs.enter();
+  let (caught_exc, caught_msg) = {
+    let mut try_catch = v8::TryCatch::new(scope);
+    let try_catch = try_catch.enter();
+    // Throw exception.
+    let msg = v8::String::new(scope, "DANG!").unwrap();
+    let exc = v8::Exception::type_error(scope, msg);
+    scope.isolate().throw_exception(exc);
+    // Catch exception.
+    let caught_exc = try_catch.exception(scope).unwrap();
+    let caught_msg = try_catch.message(scope).unwrap();
+    // Move `caught_exc` and `caught_msg` out of the extent of the TryCatch,
+    // but still within the extent of the enclosing HandleScope.
+    (caught_exc, caught_msg)
+  };
+  // This should not crash.
+  assert!(caught_exc
+    .to_string(scope)
+    .unwrap()
+    .to_rust_string_lossy(scope)
+    .contains("DANG"));
+  assert!(caught_msg
+    .get(scope)
+    .to_rust_string_lossy(scope)
+    .contains("DANG"));
+}
+
 #[test]
 fn throw_exception() {
   let _setup_guard = setup();
@@ -591,7 +629,7 @@ fn throw_exception() {
       scope.isolate().throw_exception(exception.into());
       assert!(tc.has_caught());
       assert!(tc
-        .exception()
+        .exception(scope)
         .unwrap()
         .strict_equals(v8_str(scope, "boom").into()));
     };
@@ -1605,7 +1643,7 @@ fn module_instantiation_failures1() {
       assert!(result.is_none());
       assert!(tc.has_caught());
       assert!(tc
-        .exception()
+        .exception(scope)
         .unwrap()
         .strict_equals(v8_str(scope, "boom").into()));
       assert_eq!(v8::ModuleStatus::Uninstantiated, module.get_status());