From b36b114d061d76df9e9cc41982bb5718ac608c04 Mon Sep 17 00:00:00 2001 From: Dan Carney Date: Fri, 9 Oct 2026 12:58:26 +0000 Subject: [PATCH 1/3] Give each Worker isolate, including the snapshot zygote, a fresh isolate group 50c66ec0a switched makeWorkerIsolate() to jsg::newIsolateGroup(), but the startup-snapshot pipeline (0f395f814) reintroduced v8::IsolateGroup::GetDefault() while restructuring that function. On builds with multiple pointer-compression cages this put every Worker in the default group, sharing one cage and its heap limit. Restoring newIsolateGroup() alone would leave the zygote inconsistent: the SnapshotCreator(CreateParams) constructor always allocates its isolate in the default group, while the ArrayBuffer allocator is created for the requested group. Allocate the isolate in the requested group and pass it to the non-owning SnapshotCreator constructor instead. That creator only exits the isolate on destruction, so ~IsolateBase() now always disposes the isolate itself after releasing the creator. workerd's own build does not enable multiple cages, so newIsolateGroup() returns the default group here and behavior is unchanged locally. Addresses review comments on #6908. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/workerd/jsg/setup.c++ | 22 ++++++++++------------ src/workerd/server/server.c++ | 2 +- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/workerd/jsg/setup.c++ b/src/workerd/jsg/setup.c++ index 1b8d4944d06..bf1b67c3d64 100644 --- a/src/workerd/jsg/setup.c++ +++ b/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(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(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(); diff --git a/src/workerd/server/server.c++ b/src/workerd/server/server.c++ index 3427b53a22b..1f2e35af8e9 100644 --- a/src/workerd/server/server.c++ +++ b/src/workerd/server/server.c++ @@ -5907,7 +5907,7 @@ kj::Own Server::makeWorkerIsolate(kj::StringPtr name, auto jsgobserver = kj::atomicRefcounted(); auto observer = kj::atomicRefcounted(); auto limitEnforcer = kj::refcounted(); - auto isolateGroup = v8::IsolateGroup::GetDefault(); + auto isolateGroup = jsg::newIsolateGroup(); kj::Array listeners; KJ_IF_SOME(l, inboundListeners.find(inboundListenersKey)) { From b97c391b8d5bbb1b3a75a2b6024b710b8eb0bee5 Mon Sep 17 00:00:00 2001 From: Dan Carney Date: Fri, 9 Oct 2026 12:58:27 +0000 Subject: [PATCH 2/3] Pass drained Rust resource templates across the FFI as ffi::Global realm_take_resource_templates() returned each persistent handle as a bare usize, which worker.c++ reconstituted into a v8::Global with memcpy. src/rust/AGENTS.md requires V8 handles to cross the bridge as the shared jsg::v8::ffi types so both sides agree on one cxx-verified definition. Return Vec using the shared struct from v8.rs, and recover each handle on the C++ side with global_from_ffi(). The lib.rs bridge now includes v8.rs.h, so its generated code depends on v8.rs@cxx. Addresses a review comment on #7668. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/rust/jsg/BUILD.bazel | 9 ++++++++- src/rust/jsg/lib.rs | 10 ++++++---- src/rust/jsg/resource.rs | 12 ++++++------ src/rust/jsg/v8.rs | 14 ++++++++------ src/workerd/io/worker.c++ | 10 ++++------ 5 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/rust/jsg/BUILD.bazel b/src/rust/jsg/BUILD.bazel index 7a3b3cd9dd4..75c0d9d48a7 100644 --- a/src/rust/jsg/BUILD.bazel +++ b/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 = [ diff --git a/src/rust/jsg/lib.rs b/src/rust/jsg/lib.rs index 8f6f5369adb..026fbbc3dc2 100644 --- a/src/rust/jsg/lib.rs +++ b/src/rust/jsg/lib.rs @@ -57,17 +57,19 @@ mod ffi { unsafe fn realm_create(isolate: *mut Isolate, feature_flags_data: &[u8]) -> Box; /// 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`; ownership transfers to the caller, which must + /// pipeline. Each returned handle is a `v8::Global`, 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; + fn realm_take_resource_templates(realm: &mut Realm) -> Vec; } 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 { +fn realm_take_resource_templates(realm: &mut Realm) -> Vec { realm.resources.take_template_handles() } diff --git a/src/rust/jsg/resource.rs b/src/rust/jsg/resource.rs index 8322a3973e6..c26c644f9de 100644 --- a/src/rust/jsg/resource.rs +++ b/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`) 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 { + /// Drains every cached resource-template handle, transferring ownership of each + /// `v8::Global` 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 { self.templates .drain() - .map(|(_, global)| global.into_raw_handle_for_snapshot()) + .map(|(_, global)| global.into_ffi_for_snapshot()) .collect() } } diff --git a/src/rust/jsg/v8.rs b/src/rust/jsg/v8.rs index 2b57b88dfa4..4ae7958db18 100644 --- a/src/rust/jsg/v8.rs +++ b/src/rust/jsg/v8.rs @@ -3029,24 +3029,26 @@ impl From for Global { } impl Global { - /// Consumes the `Global`, returning the raw persistent-handle word (bit-identical to a - /// C++ `v8::Global`) 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 } } diff --git a/src/workerd/io/worker.c++ b/src/workerd/io/worker.c++ index 8de8a6d3616..88e31252937 100644 --- a/src/workerd/io/worker.c++ +++ b/src/workerd/io/worker.c++ @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -39,7 +40,6 @@ #include #include -#include #include #include #include @@ -2412,11 +2412,9 @@ Worker::Worker(kj::Own scriptParam, kj::Vector> 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 handle; - static_assert(sizeof(handle) == sizeof(word), "v8::Global must be one pointer word"); - memcpy(static_cast(&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(kj::mv(handle))); } } auto contextGlobal = jsContext->extractContextGlobalForSnapshot(); From d3f73e859274a3718e3818a8273cacb1ce31dab8 Mon Sep 17 00:00:00 2001 From: Dan Carney Date: Fri, 9 Oct 2026 12:58:27 +0000 Subject: [PATCH 3/3] Correct context pointer slot 4 in the JSG README Slot 4 is ContextPointerSlot::BOOTSTRAP_STATE. The Rust realm lives in the isolate's SET_DATA_RUST_REALM slot, not in a context slot. Addresses a review comment on #7671. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/workerd/jsg/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/workerd/jsg/README.md b/src/workerd/jsg/README.md index 4b89fc5fa38..c36541f3c9c 100644 --- a/src/workerd/jsg/README.md +++ b/src/workerd/jsg/README.md @@ -513,7 +513,7 @@ Both may take additional `TypeHandler&` 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