Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 11 additions & 17 deletions docs/jsg.md
Original file line number Diff line number Diff line change
Expand Up @@ -2241,31 +2241,25 @@ if (jsg::HeapTracer::isInCppgcDestructor()) {
### Wrapper Lifecycle

```
1. Wrappable created (no JS wrapper yet)
1. C++ object created (no JS wrapper yet)
|
2. Wrappable passed to JavaScript
2. Object passed to JavaScript
|
3. attachWrapper() attaches the JS wrapper, allocating a cppgc shim
(or reusing one from the freelist)
3. attachWrapper() creates JS wrapper
|
4. wrapper -> shim -> Wrappable:
- the wrapper keeps the shim alive, via V8's CppHeap pointer table
- the shim holds a strong kj::Own<Wrappable>
- so the Wrappable cannot be destroyed while a wrapper exists
4. JS wrapper and C++ object linked
|
5. GC may collect the wrapper if:
5. GC may collect wrapper if:
- No JS references exist
- No strong Ref<T>s exist (one would root the wrapper)
- No strong Ref<T>s exist
- Wrapper is "unmodified"
|
6. detachWrapper() runs when the wrapper goes away, from:
- ~CppgcShim, after a major GC collected the wrapper
- ResetRoot(), when V8 drops an unmodified droppable wrapper
- clearWrappers(), at isolate shutdown
It releases the shim's reference to the Wrappable.
6. If wrapper collected but C++ object still alive:
- New wrapper created on next JS access
|
7. If other C++ references remain, the Wrappable lives on and a new
wrapper is created on the next JS access. Otherwise it is destroyed.
7. When C++ object destroyed:
- detachWrapper() called
- JS wrapper becomes empty shell
```

### Async Destructor Safety
Expand Down
162 changes: 0 additions & 162 deletions src/workerd/jsg/condemned-wrapper-test.c++

This file was deleted.

108 changes: 108 additions & 0 deletions src/workerd/jsg/isolate-shutdown-test.c++
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright (c) 2026 Cloudflare, Inc.
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
// https://opensource.org/licenses/Apache-2.0

#include "jsg-test.h"

namespace workerd::jsg::test {
namespace {

// This test covers a shutdown failure involving a wrapped parent that owns an unwrapped child.
// RefHolder is returned to JavaScript and therefore has a JS wrapper and a cppgc shim. Child is
// never exposed to JavaScript, so it has no wrapper of its own, but tracing RefHolder propagates
// the isolate pointer to it through RefHolder::visitForGc().
//
// A major GC can condemn RefHolder's wrapper while leaving its cppgc finalizer pending. Isolate
// teardown must finalize RefHolder while the isolate lock is held so that releasing its Ref<Child>
// can destroy Child immediately. If RefHolder instead remains owned by its condemned shim until
// v8::Isolate::Dispose(), its finalizer runs after the deferred-destruction queue has transitioned
// to DROPPED. Child still remembers the isolate and sees that the isolate lock is not held, so it
// tries to enqueue its destruction and fails the queueState == ACTIVE requirement.
//
// Production stacks reached the same path through container objects such as Response and R2
// GetResult releasing ReadableStream references from CppgcShim::~CppgcShim().
V8System v8System({"--expose-gc"_kj});

class ContextGlobalObject: public Object, public ContextGlobal {};

uint childDestructions = 0;
uint holderDestructions = 0;

class Child final: public Object {
public:
~Child() noexcept(false) {
++childDestructions;
}

JSG_RESOURCE_TYPE(Child) {}
};

class RefHolder final: public Object {
public:
explicit RefHolder(Ref<Child> child): child(kj::mv(child)) {}

~RefHolder() noexcept(false) {
++holderDestructions;
}

void visitForGc(GcVisitor& visitor) {
visitor.visit(child);
}

JSG_RESOURCE_TYPE(RefHolder) {}

private:
Ref<Child> child;
};

struct ShutdownContext: public ContextGlobalObject {
Ref<RefHolder> makeHolder(Lock& js) {
return js.alloc<RefHolder>(js.alloc<Child>());
}

JSG_RESOURCE_TYPE(ShutdownContext) {
JSG_NESTED_TYPE(Child);
JSG_NESTED_TYPE(RefHolder);
JSG_METHOD(makeHolder);
}
};
JSG_DECLARE_ISOLATE_TYPE(ShutdownIsolate, ShutdownContext, Child, RefHolder);

KJ_TEST("isolate shutdown finalizes condemned wrappers containing unwrapped children") {
setPredictableModeForTest();
childDestructions = 0;
holderDestructions = 0;

{
ShutdownIsolate isolate(v8System, kj::heap<IsolateObserver>());
isolate.runInLockScope([&](ShutdownIsolate::Lock& lock) {
JSG_WITHIN_CONTEXT_SCOPE(
lock, lock.newContext<ShutdownContext>().getHandle(lock), [&](Lock& js) {
js.withinHandleScope([&] {
// Drop the only JS reference while the nested handle scope ensures no temporary V8
// handle keeps RefHolder's wrapper alive.
auto source = "let holder = makeHolder(); holder = null;"_kj;
auto script = check(v8::Script::Compile(js.v8Context(), js.str(source)));
check(script->Run(js.v8Context()));
});

// Forced test GCs normally sweep atomically. Leave sweeping pending to reproduce the state
// created by a natural major GC immediately before isolate shutdown.
js.requestGcWithDeferredSweepForTesting();
// Prove that neither cppgc finalization nor child destruction happened during the GC. This
// keeps the test focused on shutdown rather than the ordinary atomic-sweep path.
KJ_ASSERT(holderDestructions == 0, holderDestructions);
KJ_ASSERT(childDestructions == 0, childDestructions);
});
});

// The isolate is destroyed with RefHolder's cppgc finalizer still pending. Isolate teardown
// must finalize the holder and release its unwrapped child safely.
}

KJ_ASSERT(holderDestructions == 1, holderDestructions);
KJ_ASSERT(childDestructions == 1, childDestructions);
}

} // namespace
} // namespace workerd::jsg::test
11 changes: 4 additions & 7 deletions src/workerd/jsg/jsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -1668,7 +1668,7 @@ Ref<T> _jsgThis(T* obj) {
// use-after-free.
//
// - tryAddRef(js) answers "is the object still usable from JS?". It requires the isolate
// lock and returns kj::none for condemned objects (see Wrappable::isCondemned()).
// lock and returns kj::none for condemned objects (see Wrappable::wasTracedInLastGc()).
// Any JS-facing work through a WeakRef must go through tryAddRef().
//
// Use operator->() for convenient single-expression access that asserts liveness:
Expand Down Expand Up @@ -1772,7 +1772,7 @@ class WeakRef {

// Try to promote to a strong Ref<T>. Returns kj::none if the target has been destroyed,
// or if the target's V8 wrapper died in a major GC whose deferred cleanup has not yet
// released the target (detected via Wrappable::isCondemned();
// released the target (detected via the GC epoch check in Wrappable::wasTracedInLastGc();
// see the implementation in setup.h). In the latter case the target is condemned and this
// WeakRef is permanently invalidated.
kj::Maybe<Ref<T>> tryAddRef(Lock&) const;
Expand Down Expand Up @@ -3115,16 +3115,13 @@ class Lock {
void requestGcForTesting() const;

// Like requestGcForTesting(), but leaves cppgc's sweep pending rather than running it inside the
// collection. On return, wrappers unreachable at the start of the GC have been collected and
// their Wrappables condemned (see Wrappable::isCondemned()), but the ~CppgcShim that releases
// each Wrappable has not run yet. This is the state a natural major GC leaves behind, and the
// only state in which the condemned-wrapper hazard is observable.
// collection. This reproduces the delayed finalization performed by a natural major GC.
//
// Pair with finishDeferredSweepForTesting() to close the window. Testing only.
void requestGcWithDeferredSweepForTesting() const;

// Completes a sweep left pending by requestGcWithDeferredSweepForTesting(), running the deferred
// ~CppgcShim finalizers. Testing only.
// cppgc finalizers. Testing only.
void finishDeferredSweepForTesting() const;

// Runs the given function synchronously with a v8::HandleScope on the stack.
Expand Down
Loading
Loading