fix(ext/node): trace v8 serializer delegate#35132
Merged
Merged
Conversation
Every node:v8 Serializer leaked because the Rust SerializerDelegate held a strong v8::Global back to the JS Serializer instance while the cppgc trace() impl was empty. V8 kept the JS instance alive via the strong global root and cppgc had no edge to walk back, so neither side could collect and the retained ValueSerializer pinned the serialized value graph forever. This is the symmetric counterpart of the deserializer cycle fixed in #34981. Share an Rc<SerializerDelegateState> between the cppgc wrapper and the delegate, store the JS object as a v8::TracedReference, and walk it from trace(). Co-Authored-By: Divy Srivastava <[email protected]>
divybot
marked this pull request as ready for review
June 11, 2026 11:24
The previous run's `test specs (1/2) debug macos-aarch64` failed because the test harness's local npm registry / esbuild server errored on the runner (`npm registry server error`, `failed to send build request to esbuild`, `AlertReceived(UnknownCA)`), failing a batch of unrelated npm/bundle specs. None touch v8 serialization; this empty commit retriggers CI on fresh runners. Co-Authored-By: Divy Srivastava <[email protected]>
littledivy
approved these changes
Jun 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every `node:v8`
Serializerconstructed leaks. The RustSerializerDelegateheld a strongv8::Global<Object>back to the JSSerializerinstance, while the cppgctrace()impl onSerializer<'_>was empty. V8 saw the JS instance as alive (via the strong global root); cppgc had no edge to walk back to break the cycle. Neither side could collect, so the retainedv8::ValueSerializerkept the user's serialized value graph alive forever.This is the symmetric counterpart of the deserializer cycle fixed by #34981. The same pattern applies directly: share an
Rc<SerializerDelegateState>between the cppgc wrapper and the delegate, store the JS object as av8::TracedReference, and walk it fromtrace().Adds a regression test (
Serializer keeps delegate alive across GC) mirroring the deserializer one: it constructs a customDefaultSerializersubclass, forces GC betweenwriteHeader()andwriteValue(), and asserts the_writeHostObjectdelegate path still runs and round-trips.Closes #35074
Closes denoland/divybot#561