Repository navigation
Rust JSG: bound jsg_traced!(<T> X<T>) by HoldsNoGcReferences - #8401
littledivy wants to merge 1 commit into
Conversation
The generic form of `jsg_traced!` gave `X<T>` a no-op `trace` and `HoldsNoGcReferences` for any `T`. `IoOwned<T>` uses it, and `IoContext::add_object` takes any `Send` value, so an `IoOwned` or `jsg_traced!` wrapper holding a `TracedGlobal` or `jsg::Rc` was never visited. The handle stayed strong, and a cycle through it was never collected: a resource holding `Slot<TracedGlobal>` that points at its own wrapper is never dropped, while the same cycle through a traced field is. In this patch the generic form bounds every parameter by `HoldsNoGcReferences`, so `X<T>` claims no GC references only where `T` holds none, and `add_object` requires `T: HoldsNoGcReferences`. `Weak<R>`, which holds no reference to trace whatever `R` is, implements both traits by hand. The values the request owns today (the span wrapper and the memory cache's fallback permit) are marked with `jsg_traced!`. Compile tests cover both bounds. This is a compile-time change only.
There was a problem hiding this comment.
✅ Approved — A compile-time-only fix that tightens the GC-reference bounds correctly. Send is still implied through Traced, and no issues were found.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
The change only tightens compile-time bounds and weakens no boundary.
📖 AGENTS.md: ✅ LGTM
The src/rust/AGENTS.md edit is accurate.
📋 Code Quality: ✅ LGTM
The macro bounds, the Weak<R> impls, the add_object callers and the new compile tests are all consistent. Send is still enforced, because HoldsNoGcReferences is a subtrait of Traced, which requires Send.
⚡ Performance: ✅ LGTM
The change is compile-time only and has no runtime cost.
📝 Documentation: ✅ LGTM
The doc comments and the src/rust/AGENTS.md update match the new bounds.
Reviewed commit: d3ac2b18
The generic form of
jsg_traced!gaveX<T>a no-optraceandHoldsNoGcReferencesfor anyT.IoOwned<T>uses it, andIoContext::add_objecttakes anySendvalue. So anIoOwned, or anyjsg_traced!wrapper, that holds aTracedGlobalorjsg::Rcwas never visited. The handle stayed strong, and a cycle through it was never collected. A resource that holds aSlot<TracedGlobal>pointing back at its own wrapper is never dropped, while the same cycle through a traced field is.In this patch:
HoldsNoGcReferences, soX<T>claims no GC references only whereTholds none.Weak<R>holds nothing to trace whateverRis, so it implements both traits by hand.add_objectrequiresT: HoldsNoGcReferences. The values the request owns today (the span wrapper and the memory cache's fallback permit) are marked withjsg_traced!.