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(); 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 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)) {