diff --git a/src/try_catch.rs b/src/try_catch.rs index 485d5fefda..e132a839c4 100644 --- a/src/try_catch.rs +++ b/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> { - 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> { + 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> { + 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> { - 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. diff --git a/tests/compile_fail/try_catch_exception_lifetime.rs b/tests/compile_fail/try_catch_exception_lifetime.rs index 3b07e5db7f..a77835c851 100644 --- a/tests/compile_fail/try_catch_exception_lifetime.rs +++ b/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() }; } diff --git a/tests/compile_fail/try_catch_exception_lifetime.stderr b/tests/compile_fail/try_catch_exception_lifetime.stderr index 856b887ff4..655e562b48 100644 --- a/tests/compile_fail/try_catch_exception_lifetime.stderr +++ b/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 diff --git a/tests/compile_fail/try_catch_message_lifetime.rs b/tests/compile_fail/try_catch_message_lifetime.rs index 58025798ef..e64b6d0822 100644 --- a/tests/compile_fail/try_catch_message_lifetime.rs +++ b/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() }; } diff --git a/tests/compile_fail/try_catch_message_lifetime.stderr b/tests/compile_fail/try_catch_message_lifetime.stderr index 2e4594f2f1..d9aee186d2 100644 --- a/tests/compile_fail/try_catch_message_lifetime.stderr +++ b/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 diff --git a/tests/test_api.rs b/tests/test_api.rs index 06f55ce4db..db521f216e 100644 --- a/tests/test_api.rs +++ b/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());