Cleanup after from the startup-snapshot and WrapGlobal changes

src/rust/jsg/BUILD.bazel

@@ -3,7 +3,14 @@ load("//:build/wd_rust_crate.bzl", "wd_rust_crate")
 
 wd_rust_crate(
     name = "jsg",
-    cxx_bridge_deps = ["//src/workerd/jsg:jsg-core"],
+    cxx_bridge_deps = {
+        # lib.rs reuses the shared V8 handle structs generated from v8.rs.
+        "lib.rs": [
+            ":v8.rs@cxx",
+            "//src/workerd/jsg:jsg-core",
+        ],
+        "v8.rs": ["//src/workerd/jsg:jsg-core"],
+    },
     cxx_bridge_features = ["-parse_headers"],
     cxx_bridge_local_defines = ["JSG_IMPLEMENTATION"],
     cxx_bridge_srcs = [

src/rust/jsg/lib.rs

@@ -57,17 +57,19 @@ mod ffi {
         unsafe fn realm_create(isolate: *mut Isolate, feature_flags_data: &[u8]) -> Box<Realm>;
 
         /// Drains the Realm's cached resource-template handles for the `PREPARE_SNAPSHOT`
-        /// pipeline. Each returned word is a raw persistent handle bit-identical to a C++
-        /// `v8::Global<v8::FunctionTemplate>`; ownership transfers to the caller, which must
+        /// pipeline. Each returned handle is a `v8::Global<v8::FunctionTemplate>`, recovered on
+        /// the C++ side with `global_from_ffi`; ownership transfers to the caller, which must
         /// dispose the handle before `CreateBlob`. The cache is left empty (templates are
         /// recreated lazily; `START_FROM_SNAPSHOT` isolates start empty anyway).
-        fn realm_take_resource_templates(realm: &mut Realm) -> Vec<usize>;
+        fn realm_take_resource_templates(realm: &mut Realm) -> Vec<Global>;
     }
 
     unsafe extern "C++" {
         include!("workerd/rust/jsg/ffi.h");
+        include!("workerd/rust/jsg/v8.rs.h");
 
         type Isolate = crate::v8::ffi::Isolate;
+        type Global = crate::v8::ffi::Global;
 
         // Realm
         pub unsafe fn realm_from_isolate(isolate: *mut Isolate) -> *mut Realm;
@@ -989,7 +991,7 @@ unsafe fn realm_create(isolate: *mut v8::ffi::Isolate, feature_flags_data: &[u8]
 
 /// See the bridge declaration: drains the cached resource-template persistent handles for
 /// the `PREPARE_SNAPSHOT` pipeline.
-fn realm_take_resource_templates(realm: &mut Realm) -> Vec<usize> {
+fn realm_take_resource_templates(realm: &mut Realm) -> Vec<v8::ffi::Global> {
     realm.resources.take_template_handles()
 }
 

src/rust/jsg/resource.rs

@@ -484,14 +484,14 @@ impl Resources {
         }
     }
 
-    /// Drains every cached resource-template handle, transferring ownership of the raw
-    /// persistent-handle words (bit-identical to C++ `v8::Global<v8::FunctionTemplate>`) to
-    /// the caller. Used by the `PREPARE_SNAPSHOT` pipeline, which disposes each handle before
-    /// `CreateBlob`; the cache is left empty and templates are recreated lazily on demand.
-    pub fn take_template_handles(&mut self) -> Vec<usize> {
+    /// Drains every cached resource-template handle, transferring ownership of each
+    /// `v8::Global<v8::FunctionTemplate>` to the caller. Used by the `PREPARE_SNAPSHOT`
+    /// pipeline, which disposes each handle before `CreateBlob`; the cache is left empty and
+    /// templates are recreated lazily on demand.
+    pub fn take_template_handles(&mut self) -> Vec<v8::ffi::Global> {
         self.templates
             .drain()
-            .map(|(_, global)| global.into_raw_handle_for_snapshot())
+            .map(|(_, global)| global.into_ffi_for_snapshot())
             .collect()
     }
 }

src/rust/jsg/v8.rs

@@ -3029,24 +3029,26 @@ impl<T> From<ffi::Global> for Global<T> {
 }
 
 impl<T> Global<T> {
-    /// Consumes the `Global`, returning the raw persistent-handle word (bit-identical to a
-    /// C++ `v8::Global<T>`) and transferring ownership to the caller, which becomes
-    /// responsible for disposing the handle. Snapshot-pipeline use only.
+    /// Consumes the `Global`, returning its strong FFI handle and transferring ownership to
+    /// the caller, which becomes responsible for disposing the handle. Snapshot-pipeline use
+    /// only.
     ///
     /// # Panics
     ///
     /// Panics if a traced reference has been installed on this `Global`; cached resource
     /// templates are never GC-traced, so this does not happen for them.
-    pub fn into_raw_handle_for_snapshot(self) -> usize {
+    pub fn into_ffi_for_snapshot(self) -> ffi::Global {
         // SAFETY: reading the UnsafeCell is sound — we hold the only reference.
         let traced_ptr = unsafe { (*self.traced.get()).ptr };
         assert_eq!(
             traced_ptr, 0,
             "cannot transfer a Global with an active traced reference"
         );
-        let ptr = self.handle.ptr;
+        let handle = ffi::Global {
+            ptr: self.handle.ptr,
+        };
         std::mem::forget(self);
-        ptr
+        handle
     }
 }
 

src/workerd/io/worker.c++

@@ -27,6 +27,7 @@
 #include <workerd/jsg/script.h>
 #include <workerd/jsg/setup.h>
 #include <workerd/jsg/util.h>
+#include <workerd/rust/jsg/ffi-inl.h>
 #include <workerd/rust/jsg/lib.rs.h>
 #include <workerd/rust/jsg/v8.rs.h>
 #include <workerd/util/autogate.h>
@@ -39,7 +40,6 @@
 #include <workerd/util/uuid.h>
 #include <workerd/util/xthreadnotifier.h>
 
-#include <rust/jsg/ffi.h>
 #include <v8-inspector.h>
 #include <v8-profiler.h>
 #include <v8-wasm.h>
@@ -2412,11 +2412,9 @@ Worker::Worker(kj::Own<const Script> scriptParam,
       kj::Vector<v8::Global<v8::FunctionTemplate>> rustTemplateHandles;
       {
         auto* realm = ::workerd::rust::jsg::realm_from_isolate(lock.v8Isolate);
-        for (size_t word: ::workerd::rust::jsg::realm_take_resource_templates(*realm)) {
-          v8::Global<v8::FunctionTemplate> handle;
-          static_assert(sizeof(handle) == sizeof(word), "v8::Global must be one pointer word");
-          memcpy(static_cast<void*>(&handle), &word, sizeof(word));
-          rustTemplateHandles.add(kj::mv(handle));
+        for (auto& handle: ::workerd::rust::jsg::realm_take_resource_templates(*realm)) {
+          rustTemplateHandles.add(
+              ::workerd::rust::jsg::global_from_ffi<v8::FunctionTemplate>(kj::mv(handle)));
         }
       }
       auto contextGlobal = jsContext->extractContextGlobalForSnapshot();

src/workerd/jsg/README.md

@@ -513,7 +513,7 @@ Both may take additional `TypeHandler<T>&` trailing parameters.
 | 1    | `MODULE_REGISTRY`          | Pointer to module registry        |
 | 2    | `EXTENDED_CONTEXT_WRAPPER` | Extended type wrapper for context |
 | 3    | `VIRTUAL_FILE_SYSTEM`      | Virtual file system               |
-| 4    | `RUST_REALM`               | Rust realm pointer                |
+| 4    | `BOOTSTRAP_STATE`          | Pointer to bootstrap state        |
 
 ## Wrappable Lifecycle
 

src/workerd/jsg/setup.c++

@@ -428,8 +428,11 @@ IsolateWithSnapshotCreator newIsolateWithSnapshotCreator(v8::Isolate::CreatePara
           artifact.externalReferences.asPtr().fill(0);
           params.external_references = artifact.externalReferences.begin();
 
-          auto creator = kj::heap<v8::SnapshotCreator>(params);
-          v8::Isolate* isolate = creator->GetIsolate();
+          // The SnapshotCreator(params) constructor always allocates in the default group, so
+          // allocate the isolate in `group` ourselves. The creator does not take ownership;
+          // ~IsolateBase() disposes the isolate.
+          v8::Isolate* isolate = v8::Isolate::Allocate(group);
+          auto creator = kj::heap<v8::SnapshotCreator>(isolate, params);
           return IsolateWithSnapshotCreator{isolate, kj::mv(creator)};
         }
         KJ_CASE_ONEOF(finalizedSnapshot, FinalizedSnapshot) {
@@ -605,16 +608,11 @@ IsolateBase::~IsolateBase() noexcept(false) {
   jsg::runInV8Stack([&](jsg::V8StackScope& stackScope) {
     // Terminate the v8::platform's task queue associated with this isolate
     v8System.shutdownIsolate(ptr);
-    // When preparing a snapshot the v8::SnapshotCreator owns the isolate and keeps it "entered" by
-    // the current thread; v8::Isolate::Dispose() refuses to run on an entered isolate. Destroy the
-    // SnapshotCreator first — its destructor exits and disposes the isolate — and skip
-    // ptr->Dispose() in that case.
-    if (isPreparingSnapshot()) {
-      // Destroying the SnapshotCreator exits and disposes its isolate.
-      snapshotCreator = kj::none;
-    } else {
-      ptr->Dispose();
-    }
+    // When preparing a snapshot the v8::SnapshotCreator keeps the isolate "entered" by the current
+    // thread, and v8::Isolate::Dispose() refuses to run on an entered isolate. Destroying the
+    // SnapshotCreator exits the isolate; it does not dispose it, since we allocated the isolate.
+    snapshotCreator = kj::none;
+    ptr->Dispose();
     ptr = nullptr;
     // TODO(cleanup): meaningless after V8 13.4 is released.
     cppHeap.reset();

src/workerd/server/server.c++

@@ -5907,7 +5907,7 @@ kj::Own<Worker::Isolate> Server::makeWorkerIsolate(kj::StringPtr name,
   auto jsgobserver = kj::atomicRefcounted<JsgIsolateObserver>();
   auto observer = kj::atomicRefcounted<IsolateObserver>();
   auto limitEnforcer = kj::refcounted<NullIsolateLimitEnforcer>();
-  auto isolateGroup = v8::IsolateGroup::GetDefault();
+  auto isolateGroup = jsg::newIsolateGroup();
 
   kj::Array<Worker::Api::InboundListener> listeners;
   KJ_IF_SOME(l, inboundListeners.find(inboundListenersKey)) {