Add jsg::Cell and jsg::RefCell for traced fields

docs/reference/rust-review-checklist.md

@@ -82,14 +82,12 @@ Rust types exposed to JavaScript via the JSG bindings follow these patterns:
   Methods with a `&self` receiver are registered as instance methods on the prototype; methods
   without a receiver are registered as static methods on the constructor. Any other receiver is a
   compile error, since JavaScript can call back into the resource while a method runs; state that
-  methods change belongs in `Cell` or `RefCell` fields. A method or accessor can take
-  `this: jsg::Rc<Self>` instead of `&self` to get a `jsg::Rc` of its resource, for example to
-  hand to a callback. Flag state that a method shares with a callback through an `std::rc::Rc`;
+  methods change belongs in `Cell` or `RefCell` fields or, when it holds a `jsg::Rc` or a
+  `TracedGlobal`, in `jsg::Cell` or `jsg::RefCell` fields (`src/rust/jsg/README.md` §Cells). A
+  method or accessor can take `this: jsg::Rc<Self>` instead of `&self` to get a `jsg::Rc` of its
+  resource, for example to hand to a callback. Flag state that a method shares with a callback through an `std::rc::Rc`;
   the callback should hold `this` (or `this.downgrade()`) instead. Flag a
   `RefCell` borrow held across a call that can run JavaScript: a re-entrant call then panics.
-  Also flag a `borrow_mut()` of a `RefCell` holding GC references (`TracedGlobal`, `jsg::Rc`)
-  held across a call that can allocate on the V8 heap, wrap the resource or create or drop a
-  `jsg::Rc` to it: tracing skips the cell, so those references can lose their values.
   Verify the converted name is correct and matches the intended API surface.
 - **`#[jsg_static_constant]`** on a `const` item inside a `#[jsg_resource]` impl block exposes it
   as a read-only numeric constant on both the constructor and prototype (Rust equivalent of
@@ -152,10 +150,12 @@ some violations, and the README says which. Reviewers flag these:
 - An `std::rc::Rc` around a `TracedGlobal`, a `jsg::Rc`, or a type holding them, even if it is
   not shared yet. The fix is a `jsg::Rc` or `jsg::Weak` to the resource that owns the state,
   such as a method's `this`.
-- A `jsg::Rc` moved out of a traced field (`Cell::take`, `Option::take`, `mem::replace`, popping
-  a traced `Vec`) and used after a call that can run JavaScript or allocate on the V8 heap,
-  without a `.clone()` first
-  ([#7976](https://github.com/cloudflare/workerd/issues/7976)).
+- A `jsg::Rc` or `TracedGlobal` reached through interior mutability other than a `jsg::Cell` or
+  `jsg::RefCell`: `UnsafeCell`, `OnceCell`, a hand-written cell, or a std `Cell` or `RefCell`
+  that a hand-written `impl Traced` visits through `borrow()`. A `jsg::Rc` or `TracedGlobal` in
+  a std cell field of a `#[jsg_resource]` or `#[jsg_struct]` does not compile; these are the
+  paths the compiler cannot see ([#7976](https://github.com/cloudflare/workerd/issues/7976)).
+- A hand-written `impl HoldsNoGcReferences` on a type whose `trace` visits something.
 - A `jsg::v8::Global` inside a type with a hand-written `impl Traced`.
 - New C++ JSG (`jsg::Lock`, `JSG_TRY`/`JSG_CATCH`, `jsg::check`, `JsExceptionThrown`,
   `createTunneledException`, `makeInternalError`, `jsg::Data`/`jsg::Value`) in a `.c++` or `.h`

src/rust/AGENTS.md

@@ -35,18 +35,18 @@ _Snapshot — the set drifts as crates come and go; `bazel query //src/rust/...`
 - **Namespace**: `workerd::rust::*` for crates in this directory, except `python-parser` → `edgeworker::rust::python_parser`; a crate living beside C++ uses that C++'s namespace (e.g. `workerd::server::cli`)
 - **Errors**: `thiserror` for library crates; `jsg::Error` with `ExceptionType` for JSG-facing crates
 - **JSG nullish values**: `Option<T>` is `T | undefined`; `Nullable<T>` is `T | null` (coerces `undefined` to `Null`, but method arguments remain required). Prefer `NullOrUndefined<T>` to `Option<Nullable<T>>` for three distinct `Some(T)`, `Null`, and `Undefined` states; omitted arguments become `Undefined`. `Lenient<T>` additionally preserves shallow mismatches as `Unconvertable`; converting it to `NullOrUndefined<T>` maps those to `Undefined`, and `Lenient::to_parts()` also returns the original JS value. See [Rust JSG nullish values](jsg/README.md#nullablet-and-nullorundefinedt).
-- **JSG resources**: `#[jsg_resource]` on struct + impl block; `#[jsg_method]` auto-converts `snake_case` → `camelCase`; methods with `&self` become instance methods, methods without a receiver become static methods, and any other receiver is a compile error (JavaScript can re-enter a resource, so state that methods change goes in `Cell`/`RefCell` fields; never hold a `RefCell` borrow across a call that can run JavaScript, nor a `borrow_mut()` of one holding GC references across a call that can allocate on the V8 heap, wrap the resource or create or drop a `jsg::Rc` to it, since tracing skips a mutably borrowed `RefCell`); a method or accessor can take `this: jsg::Rc<Self>` as its first parameter instead of `&self` to get a `jsg::Rc` of its resource (like `JSG_THIS`) to hand to a callback; don't share the resource's state with a callback through an `std::rc::Rc` (see `jsg/README.md` §Reaching the resource from a callback); a `#[jsg_method]` is registered with the type's `v8::Signature`, so its `&self` callback resolves the receiver without the `WORKERD_RUST_WRAPPABLE_TAG` marker check (`WrappableRc::resolve_method_receiver`); callbacks without a signature (accessors, inspect properties, wildcard interceptors) use `resolve_receiver`, which reads the marker before unwrapping so that it accepts any value; an eligible `#[jsg_method]` also gets a V8 Fast API callback (types in `jsg/fast_api.rs`) and must not call into V8; a `#[jsg_constructor]` returns `Self` or `Result<Self, E>` (an `Err` is thrown) and throws `TypeError` when called without `new`; `#[jsg_static_constant]` on `const` items exposes read-only numeric constants on both constructor and prototype (name kept as-is, no camelCase); `#[jsg_resource(nested_types(Foo, Name = Bar))]` on the impl block puts other resources' constructors on the prototype (`JSG_NESTED_TYPE`); resources integrate with GC via `Traced` + `GarbageCollected`: every named field is traced via `Traced::trace(&self.field, visitor)` and all non-traceable types use no-op `Traced` impls
+- **JSG resources**: `#[jsg_resource]` on struct + impl block; `#[jsg_method]` auto-converts `snake_case` → `camelCase`; methods with `&self` become instance methods, methods without a receiver become static methods, and any other receiver is a compile error (JavaScript can re-enter a resource, so state that methods change goes in `Cell`/`RefCell` fields, or in `jsg::Cell`/`jsg::RefCell` fields when it holds a `jsg::Rc` or a `TracedGlobal`, whose `take`/`replace`/`borrow_mut` take the lock and make the contents strong; never hold a `RefCell` borrow across a call that can run JavaScript); a method or accessor can take `this: jsg::Rc<Self>` as its first parameter instead of `&self` to get a `jsg::Rc` of its resource (like `JSG_THIS`) to hand to a callback; don't share the resource's state with a callback through an `std::rc::Rc` (see `jsg/README.md` §Reaching the resource from a callback); a `#[jsg_method]` is registered with the type's `v8::Signature`, so its `&self` callback resolves the receiver without the `WORKERD_RUST_WRAPPABLE_TAG` marker check (`WrappableRc::resolve_method_receiver`); callbacks without a signature (accessors, inspect properties, wildcard interceptors) use `resolve_receiver`, which reads the marker before unwrapping so that it accepts any value; an eligible `#[jsg_method]` also gets a V8 Fast API callback (types in `jsg/fast_api.rs`) and must not call into V8; a `#[jsg_constructor]` returns `Self` or `Result<Self, E>` (an `Err` is thrown) and throws `TypeError` when called without `new`; `#[jsg_static_constant]` on `const` items exposes read-only numeric constants on both constructor and prototype (name kept as-is, no camelCase); `#[jsg_resource(nested_types(Foo, Name = Bar))]` on the impl block puts other resources' constructors on the prototype (`JSG_NESTED_TYPE`); resources integrate with GC via `Traced` + `GarbageCollected`: every named field is traced via `Traced::trace(&self.field, visitor)` and all non-traceable types use no-op `Traced` impls
 - **Resources created by C++**: C++ creates a Rust resource through an `extern "Rust"` function of the crate that defines it, which returns the wrapper through `jsg::v8::run_ffi_callback`; C++ holds the wrapper (a `jsg::JsRef`), since it has no typed reference to a Rust resource, and checks a value's type through one that returns `jsg::resource::ffi_is::<R>`, a `kj::Maybe<bool>` that is none when an exception is pending. See `jsg/README.md` §Creating resources from C++.
 - **Custom JSG registration**: `#[jsg_resource(custom_members)]` on an impl generates member factories but leaves `impl Resource` to the author. `Resource::members(registry, flags)` uses ordinary Rust to select members, order, and property placement, and also drives RTTI under the export's flags. Factories are `<rust_name>_member()` (constant names lowercased); ordinary property factories are `<getter_name>_member(PropertyPlacement)` (`Prototype`, `Instance` or `LazyInstance`) and include the paired setter. See `jsg-macros/README.md` §Custom member registration.
 - **JSG properties**: three property macros on `#[jsg_resource]` impl blocks — `#[jsg_property(prototype|instance|lazy_instance [, name = "..."] [, readonly])]` (`prototype` registers an accessor on the prototype, equivalent to `JSG_PROTOTYPE_PROPERTY`; `instance` registers an own data property via V8 `SetNativeDataProperty`, equivalent to `JSG_INSTANCE_PROPERTY`; `lazy_instance` registers an own data property via V8 `SetLazyDataProperty` whose getter runs once, on first access, equivalent to `JSG_LAZY_INSTANCE_PROPERTY`: it has no setter, is writable unless `readonly` (`JSG_LAZY_READONLY_INSTANCE_PROPERTY`); declaration order sets `Object.keys()` order for automatic registration; `readonly` is a compile-time check preventing a paired setter; `name = "..."` overrides the JS name; prefer `prototype` in almost all cases), `#[jsg_inspect_property]` (registered under a unique symbol, invisible to normal enumeration and string-key lookup, surfaced by `node:util` `inspect()`, equivalent to `JSG_INSPECT_PROPERTY`), and `#[jsg_wildcard_property]` (answers string-named properties the object and its prototypes lack, at most one per type, equivalent to `JSG_WILDCARD_PROPERTY`; see `jsg/README.md`); setter auto-detected from `set_` prefix; read-only when no setter present; getter/setter `.length` and `.name` are set correctly when `spec_compliant_property_attributes` compat flag is enabled
-- **`jsg::v8::Global` / `TracedGlobal`**: persistent handles. A `Global` is for handles Rust owns outside resources: GC never visits it, so it is always strong, holds its handle inline (no box), has no `Traced` impl (it cannot be a resource field), and is `Send` and `Sync`. A `TracedGlobal` is for resource fields, which GC traces to collect cycles; it is `Send` (not `Sync`). Either may be dropped on any thread. Under the isolate lock its strong handle is reset at once; without the lock it is queued and reset at the next `IsolateBase::Lock`, through `IsolateBase::applyDeferredActions()`; once the `Realm` is dropped (under the lock, before `Dispose`) the handles are left to `Dispose`. See `jsg/isolate_state.rs`. Hand-written FFI that resets a V8 handle directly must hold the lock. Rust never resets a `TracedGlobal`'s traced reference: every GC visit lets go of it and makes the strong handle strong again (`ClearWeak`) before C++ installs a new one if needed, and a drop just lets go of it; V8 frees untraced nodes at the next full GC. See `GcVisitor::visit_traced_global`. A `TracedGlobal` keeps its handles in a `Box<GlobalSlot>`, since V8 holds the address of a weak handle's slot; the slot of a `TracedGlobal` dropped after the `Realm` is kept until the isolate is disposed
+- **`jsg::v8::Global` / `TracedGlobal`**: persistent handles. A `Global` is for handles Rust owns outside resources: GC never visits it, so it is always strong, holds its handle inline (no box), has no `Traced` impl (it cannot be a resource field), and is `Send` and `Sync`. A `TracedGlobal` is for resource fields, which GC traces to collect cycles; it is `Send` (not `Sync`). Either may be dropped on any thread. Under the isolate lock its strong handle is reset at once; without the lock it is queued and reset at the next `IsolateBase::Lock`, through `IsolateBase::applyDeferredActions()`; once the `Realm` is dropped (under the lock, before `Dispose`) the handles are left to `Dispose`. See `jsg/isolate_state.rs`. Hand-written FFI that resets a V8 handle directly must hold the lock. Rust never resets a `TracedGlobal`'s traced reference: every GC visit, and the detaching visit on the way out of a `jsg::` cell, lets go of it and makes the strong handle strong again (`ClearWeak`) before C++ installs a new one if needed, and a drop just lets go of it; V8 frees untraced nodes at the next full GC. See `GcVisitor::visit_traced_global`. A `TracedGlobal` keeps its handles in a `Box<GlobalSlot>`, since V8 holds the address of a weak handle's slot; the slot of a `TracedGlobal` dropped after the `Realm` is kept until the isolate is disposed
 - **JSG parameters**: function `.length` is the Web IDL required-argument count under `spec_compliant_property_attributes` (`this: jsg::Rc<Self>` and a leading `&mut Lock` are not counted; counting stops at the first type whose `FromJS::is_optional()` returns true), so put optional parameters last
-- **`Traced`**: core tracing trait in `jsg::wrappable`; built-ins include no-op impls for primitives/value types and delegating impls for wrappers/collections (`Option`, `Nullable`, `NullOrUndefined`, `Lenient`, `Vec`, maps/sets, `Cell`, `jsg::Rc`, `jsg::Weak`, `jsg::v8::TracedGlobal`); `jsg::v8::Global` deliberately has none
+- **`Traced`**: core tracing trait in `jsg::wrappable`, with `trace`, which GC calls and which also detaches a value from its owner (`GcVisitor::is_detaching`: every GC reference in it becomes strong and forgets its parent, so it stays valid outside a traced field); built-ins include no-op impls for primitives/value types and delegating impls for wrappers/collections (`Option`, `Nullable`, `NullOrUndefined`, `Lenient`, `Vec`, `jsg::Dict`, maps/sets, `Cell`, `RefCell`, `jsg::Cell`, `jsg::RefCell`, `jsg::Rc`, `jsg::Weak`, `jsg::v8::TracedGlobal`); `jsg::v8::Global` deliberately has none. `std::cell::Cell<T>` and `RefCell<T>` are `Traced` only for `T: jsg::HoldsNoGcReferences` (no `jsg::Rc` or `TracedGlobal` inside); a `jsg::Rc` or `TracedGlobal` that is moved out of a field lives in a `jsg::Cell` or `jsg::RefCell` (`jsg/cell.rs`), which detach it before it leaves the cell
 - **Sequences to JavaScript**: `Vec<T>` converts to an `Array`, numbers included (`vec![1u8, 2]` makes `[1, 2]`). A typed array or an `ArrayBuffer` is an explicit conversion of a slice: `to_js_typed_array(lock)` makes the typed array of the element type, and `to_js_array_buffer(lock)` makes an `ArrayBuffer` of bytes (`[u8]` only), as C++ JSG returns for a `kj::Array<kj::byte>`. See `jsg/README.md` §Arrays, typed arrays and `ArrayBuffer`s
 - **`#[jsg_resource(custom_trace)]`**: suppresses the auto-generated `Traced` impl so the user can write their own; `GarbageCollected` (`memory_name`), `jsg::Type`, `jsg::ToJS`, and `jsg::FromJS` are still generated
 - **Rust JSG safety rules** (full text and examples in `src/rust/jsg/README.md` §Safety rules):
   1. Never call a V8 API that takes a `v8::Local<v8::Context>` while an exception or a termination is pending: V8 either clears the exception on entry, so the script runs on, or fails a debug check and in a release build runs on with it pending. Every C++ shim that passes a context to V8 first checks `exceptionPending()` (`jsg/ffi.h`) and fails with a pending error. Never throw while execution is terminating, and never discard a `jsg::Error` from an operation that can fail with a pending error (a call, `Object::get`/`set`, a coercion, `ToJS::to_js`); pass it on with `?`. Creating strings, numbers, objects, arrays, error objects and templates is fine at any time.
-  2. Never put traced state (`TracedGlobal`, `jsg::Rc`, or a type holding them) behind `std::rc::Rc`. To give a callback or another party access, give it a `jsg::Rc` or `jsg::Weak` to the resource that owns the state (a method gets one as `this: jsg::Rc<Self>`). Clone a `jsg::Rc` right after moving it out of a traced field (`take`, `replace`, `pop`), since the moved one is neither strong nor traced ([#7976](https://github.com/cloudflare/workerd/issues/7976)). Don't put a `Global` in anything GC traces. A typed promise's reaction gets its value detached from the promise's wrapper, as a moved C++ `jsg::Ref` is; a `Traced` impl that visits through `GcVisitor::as_ffi` must handle the `None` it returns for such a visit (`GcVisitor::is_detaching()`).
+  2. Never put traced state (`TracedGlobal`, `jsg::Rc`, or a type holding them) behind `std::rc::Rc`. To give a callback or another party access, give it a `jsg::Rc` or `jsg::Weak` to the resource that owns the state (a method gets one as `this: jsg::Rc<Self>`). Keep a `jsg::Rc` or `TracedGlobal` that is moved out of a field in a `jsg::Cell` or `jsg::RefCell`, which make it strong on the way out; a plain move out of a std cell would leave it neither strong nor traced, so std cells holding one do not compile ([#7976](https://github.com/cloudflare/workerd/issues/7976)). Don't put a `Global` in anything GC traces. A typed promise's reaction gets its value detached from the promise's wrapper, as a moved C++ `jsg::Ref` is, and so does a value leaving a `jsg::` cell; a `Traced` impl that visits through `GcVisitor::as_ffi` must handle the `None` it returns for such a visit (`GcVisitor::is_detaching()`).
   3. The C++ shims call V8 directly, not C++ JSG (`JSG_TRY`, `jsg::check`, `jsg::Lock::from`, `createTunneledException`, `makeInternalError`, `jsg::Data`, ...). The one reviewed exception is `io_context_await_io_and_run()` in `src/workerd/io/io-context/ffi.c++`, which throws `jsg::JsExceptionThrown` to pass a pending exception to `IoContext::run()`; don't extend it. `//src/rust/jsg:cpp-jsg-allowlist-test` enforces this for the files in `CPP_SHIMS` in `jsg/BUILD.bazel`; add shim files outside `src/rust/jsg` there.
   4. `Local<'s>` and `Lock<'s>` stay in their scope, and futures passed to async I/O are `'static`.
 - **Formatting**: `rustfmt.toml` — `group_imports = "StdExternalCrate"`, `imports_granularity = "Item"` (one `use` per import)

src/rust/jsg-bench/lib.rs

@@ -67,7 +67,7 @@ enum NumberOrString {
 #[jsg_resource]
 struct Node {
     value: Cell<u32>,
-    child: Cell<Option<jsg::Rc<Self>>>,
+    child: jsg::Cell<Option<jsg::Rc<Self>>>,
 }
 
 #[jsg_resource]
@@ -76,7 +76,7 @@ impl Node {
     fn constructor(value: u32) -> Self {
         Self {
             value: Cell::new(value),
-            child: Cell::new(None),
+            child: jsg::Cell::new(None),
         }
     }
 
@@ -91,8 +91,8 @@ impl Node {
     }
 
     #[jsg_property(prototype)]
-    fn get_child(&self) -> Option<jsg::Rc<Node>> {
-        let child = self.child.take();
+    fn get_child(&self, lock: &mut jsg::Lock<'_>) -> Option<jsg::Rc<Node>> {
+        let child = self.child.take(lock);
         self.child.set(child.clone());
         child
     }