core: Add test for snapshotting from Rust (#2197)

5 files changed, 78 insertions, 35 deletions

diff --git a/core/isolate.rs b/core/isolate.rs
index 95cf1be17..5488fab75 100644
--- a/core/isolate.rs
+++ b/core/isolate.rs
@@ -65,12 +65,14 @@ pub struct Script<'a> {
 pub enum StartupData<'a> {
   Script(Script<'a>),
   Snapshot(&'a [u8]),
+  LibdenoSnapshot(Snapshot1<'a>),
   None,
 }
 
 #[derive(Default)]
 pub struct Config {
   dispatch: Option<Arc<Fn(&[u8], deno_buf) -> (bool, Box<Op>) + Send + Sync>>,
+  pub will_snapshot: bool,
 }
 
 impl Config {
@@ -129,21 +131,29 @@ impl Isolate {
     let shared = SharedQueue::new(RECOMMENDED_SIZE);
 
     let needs_init = true;
-    // Seperate into Option values for eatch startup type
-    let (startup_snapshot, startup_script) = match startup_data {
-      StartupData::Snapshot(d) => (Some(d), None),
-      StartupData::Script(d) => (None, Some(d)),
-      StartupData::None => (None, None),
-    };
-    let libdeno_config = libdeno::deno_config {
-      will_snapshot: 0,
-      load_snapshot: match startup_snapshot {
-        Some(s) => Snapshot2::from(s),
-        None => Snapshot2::empty(),
-      },
+
+    let mut startup_script: Option<Script> = None;
+    let mut libdeno_config = libdeno::deno_config {
+      will_snapshot: if config.will_snapshot { 1 } else { 0 },
+      load_snapshot: Snapshot2::empty(),
       shared: shared.as_deno_buf(),
       recv_cb: Self::pre_dispatch,
     };
+
+    // Seperate into Option values for each startup type
+    match startup_data {
+      StartupData::Script(d) => {
+        startup_script = Some(d);
+      }
+      StartupData::Snapshot(d) => {
+        libdeno_config.load_snapshot = d.into();
+      }
+      StartupData::LibdenoSnapshot(d) => {
+        libdeno_config.load_snapshot = d;
+      }
+      StartupData::None => {}
+    };
+
     let libdeno_isolate = unsafe { libdeno::deno_new(libdeno_config) };
 
     let mut core_isolate = Self {
@@ -342,7 +352,7 @@ impl Isolate {
     out
   }
 
-  pub fn snapshot_new(&self) -> Result<Snapshot1, JSError> {
+  pub fn snapshot(&self) -> Result<Snapshot1<'static>, JSError> {
     let snapshot = unsafe { libdeno::deno_snapshot_new(self.libdeno_isolate) };
     if let Some(js_error) = self.last_exception() {
       assert_eq!(snapshot.data_ptr, null());
@@ -994,4 +1004,21 @@ pub mod tests {
       assert_eq!(Ok(Async::Ready(())), isolate.poll());
     });
   }
+
+  #[test]
+  fn will_snapshot() {
+    let snapshot = {
+      let mut config = Config::default();
+      config.will_snapshot = true;
+      let mut isolate = Isolate::new(StartupData::None, config);
+      js_check(isolate.execute("a.js", "a = 1 + 2"));
+      let s = isolate.snapshot().unwrap();
+      drop(isolate);
+      s
+    };
+
+    let startup_data = StartupData::LibdenoSnapshot(snapshot);
+    let mut isolate2 = Isolate::new(startup_data, Config::default());
+    js_check(isolate2.execute("check.js", "if (a != 3) throw Error('x')"));
+  }
 }
diff --git a/core/libdeno.rs b/core/libdeno.rs
index e6445f299..048db1311 100644
--- a/core/libdeno.rs
+++ b/core/libdeno.rs
@@ -126,13 +126,6 @@ unsafe impl Send for deno_snapshot<'_> {}
 /// The type returned from deno_snapshot_new. Needs to be dropped.
 pub type Snapshot1<'a> = deno_snapshot<'a>;
 
-// TODO Does this make sense?
-impl Drop for Snapshot1<'_> {
-  fn drop(&mut self) {
-    unsafe { deno_snapshot_delete(self) }
-  }
-}
-
 /// The type created from slice. Used for loading.
 pub type Snapshot2<'a> = deno_snapshot<'a>;
 
diff --git a/core/libdeno/api.cc b/core/libdeno/api.cc
index ad9c2a574..53dad58e6 100644
--- a/core/libdeno/api.cc
+++ b/core/libdeno/api.cc
@@ -101,6 +101,7 @@ deno_snapshot deno_snapshot_new(Deno* d_) {
 
   auto blob = d->snapshot_creator_->CreateBlob(
       v8::SnapshotCreator::FunctionCodeHandling::kKeep);
+  d->has_snapshotted_ = true;
   return {reinterpret_cast<uint8_t*>(const_cast<char*>(blob.data)),
           blob.raw_size};
 }
diff --git a/core/libdeno/internal.h b/core/libdeno/internal.h
index ce8c63c56..0f4df9908 100644
--- a/core/libdeno/internal.h
+++ b/core/libdeno/internal.h
@@ -38,7 +38,8 @@ class DenoIsolate {
         recv_cb_(config.recv_cb),
         next_zero_copy_id_(1),  // zero_copy_id must not be zero.
         user_data_(nullptr),
-        resolve_cb_(nullptr) {
+        resolve_cb_(nullptr),
+        has_snapshotted_(false) {
     array_buffer_allocator_ = v8::ArrayBuffer::Allocator::NewDefaultAllocator();
     if (config.load_snapshot.data_ptr) {
       snapshot_.data =
@@ -53,7 +54,14 @@ class DenoIsolate {
       delete locker_;
     }
     if (snapshot_creator_) {
-      delete snapshot_creator_;
+      // TODO(ry) V8 has a strange assert which prevents a SnapshotCreator from
+      // being deallocated if it hasn't created a snapshot yet.
+      // https://github.com/v8/v8/blob/73212783fbd534fac76cc4b66aac899c13f71fc8/src/api.cc#L603
+      // If that assert is removed, this if guard could be removed.
+      // WARNING: There may be false positive LSAN errors here.
+      if (has_snapshotted_) {
+        delete snapshot_creator_;
+      }
     } else {
       isolate_->Dispose();
     }
@@ -120,6 +128,7 @@ class DenoIsolate {
   v8::StartupData snapshot_;
   v8::Persistent<v8::ArrayBuffer> global_import_buf_;
   v8::Persistent<v8::SharedArrayBuffer> shared_ab_;
+  bool has_snapshotted_;
 };
 
 class UserDataScope {
diff --git a/core/shared_queue.js b/core/shared_queue.js
index aa3ff0aee..3a447cedb 100644
--- a/core/shared_queue.js
+++ b/core/shared_queue.js
@@ -16,6 +16,8 @@ SharedQueue Binary Layout
 +---------------------------------------------------------------+
  */
 
+/* eslint-disable @typescript-eslint/no-use-before-define */
+
 (window => {
   const GLOBAL_NAMESPACE = "Deno";
   const CORE_NAMESPACE = "core";
@@ -32,6 +34,25 @@ SharedQueue Binary Layout
 
   let sharedBytes;
   let shared32;
+  let initialized = false;
+
+  function maybeInit() {
+    if (!initialized) {
+      init();
+      initialized = true;
+    }
+  }
+
+  function init() {
+    let shared = Deno.core.shared;
+    assert(shared.byteLength > 0);
+    assert(sharedBytes == null);
+    assert(shared32 == null);
+    sharedBytes = new Uint8Array(shared);
+    shared32 = new Int32Array(shared);
+    // Callers should not call Deno.core.recv, use setAsyncHandler.
+    Deno.core.recv(handleAsyncMsgFromRust);
+  }
 
   function assert(cond) {
     if (!cond) {
@@ -40,12 +61,14 @@ SharedQueue Binary Layout
   }
 
   function reset() {
+    maybeInit();
     shared32[INDEX_NUM_RECORDS] = 0;
     shared32[INDEX_NUM_SHIFTED_OFF] = 0;
     shared32[INDEX_HEAD] = HEAD_INIT;
   }
 
   function head() {
+    maybeInit();
     return shared32[INDEX_HEAD];
   }
 
@@ -121,6 +144,7 @@ SharedQueue Binary Layout
 
   let asyncHandler;
   function setAsyncHandler(cb) {
+    maybeInit();
     assert(asyncHandler == null);
     asyncHandler = cb;
   }
@@ -135,17 +159,8 @@ SharedQueue Binary Layout
     }
   }
 
-  function init(shared) {
-    assert(shared.byteLength > 0);
-    assert(sharedBytes == null);
-    assert(shared32 == null);
-    sharedBytes = new Uint8Array(shared);
-    shared32 = new Int32Array(shared);
-    // Callers should not call Deno.core.recv, use setAsyncHandler.
-    window.Deno.core.recv(handleAsyncMsgFromRust);
-  }
-
   function dispatch(control, zeroCopy = null) {
+    maybeInit();
     // First try to push control to shared.
     const success = push(control);
     // If successful, don't use first argument of core.send.
@@ -170,6 +185,4 @@ SharedQueue Binary Layout
   assert(window[GLOBAL_NAMESPACE] != null);
   assert(window[GLOBAL_NAMESPACE][CORE_NAMESPACE] != null);
   Object.assign(core, denoCore);
-
-  init(Deno.core.shared);
-})(globalThis);
+})(this);