Skip to content

Commit

Permalink
Exit HandleScope before snapshotting (denoland#4168)
Browse files Browse the repository at this point in the history
The V8 documentation explicitly states that SnapshotCreator::CreateBlob()
should not be called from within a HandleScope.

Additionally, this patch removes some non-functional error handling code
from the deno_core::Isolate::snapshot() method.
  • Loading branch information
piscisaureus committed Feb 28, 2020
1 parent 9075daa commit bc7dbfa
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 13 deletions.
24 changes: 12 additions & 12 deletions core/isolate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,24 +465,26 @@ impl Isolate {
/// ErrBox can be downcast to a type that exposes additional information about
/// the V8 exception. By default this type is CoreJSError, however it may be a
/// different type if Isolate::set_js_error_create() has been used.
pub fn snapshot(&mut self) -> Result<v8::OwnedStartupData, ErrBox> {
pub fn snapshot(&mut self) -> v8::OwnedStartupData {
assert!(self.snapshot_creator.is_some());

let v8_isolate = self.v8_isolate.as_mut().unwrap();
let js_error_create_fn = &*self.js_error_create_fn;
let last_exception = &mut self.last_exception;

let mut hs = v8::HandleScope::new(v8_isolate);
let scope = hs.enter();
self.global_context.reset(scope);
// Note: create_blob() method must not be called from within a HandleScope.
// The HandleScope created here is exited at the end of the block.
// TODO(piscisaureus): The rusty_v8 type system should enforce this.
{
let v8_isolate = self.v8_isolate.as_mut().unwrap();
let mut hs = v8::HandleScope::new(v8_isolate);
let scope = hs.enter();
self.global_context.reset(scope);
}

let snapshot_creator = self.snapshot_creator.as_mut().unwrap();
let snapshot = snapshot_creator
.create_blob(v8::FunctionCodeHandling::Keep)
.unwrap();
self.has_snapshotted = true;

check_last_exception(last_exception, js_error_create_fn).map(|_| snapshot)
snapshot
}
}

Expand Down Expand Up @@ -1160,9 +1162,7 @@ pub mod tests {
let snapshot = {
let mut isolate = Isolate::new(StartupData::None, true);
js_check(isolate.execute("a.js", "a = 1 + 2"));
let s = isolate.snapshot().unwrap();
drop(isolate);
s
isolate.snapshot()
};

let startup_data = StartupData::OwnedSnapshot(snapshot);
Expand Down
2 changes: 1 addition & 1 deletion deno_typescript/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ fn write_snapshot(
snapshot_filename: &Path,
) -> Result<(), ErrBox> {
println!("Creating snapshot...");
let snapshot = runtime_isolate.snapshot()?;
let snapshot = runtime_isolate.snapshot();
let snapshot_slice: &[u8] = &*snapshot;
println!("Snapshot size: {}", snapshot_slice.len());
fs::write(&snapshot_filename, snapshot_slice)?;
Expand Down

0 comments on commit bc7dbfa

Please sign in to comment.