Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit 64b0baa

Browse files
committed
Bug 1935738 - Implement GCPolicy<Heap<T>>::needsSweep. r=sfink,jonco
Without a needsSweep() implementation for JS::GCPolicy<JS::Heap<T>>, embedders cannot put heap pointers inside containers like JS::WeakCache. (Firefox is not affected by this because it uses HeapPtr instead, which is not available to embedders.) Differential Revision: https://phabricator.services.mozilla.com/D231442
1 parent 6d3a38b commit 64b0baa

File tree

2 files changed

+123
-0
lines changed

2 files changed

+123
-0
lines changed

js/public/GCPolicyAPI.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,10 @@ struct GCPolicy<JS::Heap<T>> {
165165
static bool traceWeak(JSTracer* trc, JS::Heap<T>* thingp) {
166166
return !*thingp || js::gc::TraceWeakEdge(trc, thingp);
167167
}
168+
static bool needsSweep(JSTracer* trc, const JS::Heap<T>* thingp) {
169+
T* thing = const_cast<T*>(thingp->unsafeAddress());
170+
return thing && js::gc::EdgeNeedsSweepUnbarrieredSlow(thing);
171+
}
168172
};
169173

170174
// GCPolicy<UniquePtr<T>> forwards the contained pointer to GCPolicy<T>.

js/src/jsapi-tests/testGCWeakCache.cpp

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,47 @@ BEGIN_TEST(testWeakCacheSet) {
5555
}
5656
END_TEST(testWeakCacheSet)
5757

58+
BEGIN_TEST(testWeakCacheSetWithJSHeap) {
59+
// Tests basically the same thing as testWeakCacheSet, but only using APIs
60+
// available to embedders.
61+
JS::RootedObject tenured1{cx, JS_NewPlainObject(cx)};
62+
JS::RootedObject tenured2{cx, JS_NewPlainObject(cx)};
63+
JS_GC(cx);
64+
JS::RootedObject nursery1{cx, JS_NewPlainObject(cx)};
65+
JS::RootedObject nursery2{cx, JS_NewPlainObject(cx)};
66+
67+
using ObjectSet = JS::GCHashSet<JS::Heap<JSObject*>,
68+
js::StableCellHasher<JS::Heap<JSObject*>>,
69+
js::SystemAllocPolicy>;
70+
using Cache = JS::WeakCache<ObjectSet>;
71+
Cache cache{JS::GetObjectZone(tenured1)};
72+
73+
cache.put(tenured1);
74+
cache.put(tenured2);
75+
cache.put(nursery1);
76+
cache.put(nursery2);
77+
cache.put(nullptr); // nullptr entries should not be swept.
78+
79+
// Verify relocation and that we don't sweep too aggressively.
80+
JS_GC(cx);
81+
CHECK(cache.has(tenured1));
82+
CHECK(cache.has(tenured2));
83+
CHECK(cache.has(nursery1));
84+
CHECK(cache.has(nursery2));
85+
CHECK(cache.has(nullptr));
86+
87+
// Unroot two entries and verify that they get removed.
88+
tenured2 = nursery2 = nullptr;
89+
JS_GC(cx);
90+
CHECK(cache.has(tenured1));
91+
CHECK(cache.has(nursery1));
92+
CHECK(cache.has(nullptr));
93+
CHECK_EQUAL(cache.count(), unsigned{3});
94+
95+
return true;
96+
}
97+
END_TEST(testWeakCacheSetWithJSHeap)
98+
5899
// Exercise WeakCache<GCHashMap>.
59100
BEGIN_TEST(testWeakCacheMap) {
60101
// Create two objects tenured and two in the nursery. If zeal is on,
@@ -125,6 +166,45 @@ BEGIN_TEST(testWeakCacheMapWithUniquePtr) {
125166
}
126167
END_TEST(testWeakCacheMapWithUniquePtr)
127168

169+
BEGIN_TEST(testWeakCacheMapWithJSHeap) {
170+
// Tests basically the same thing as testWeakCacheMap, but only using APIs
171+
// available to embedders, and with JS::Heap as both keys and values.
172+
JS::RootedObject tenured1{cx, JS_NewPlainObject(cx)};
173+
JS::RootedObject tenured2{cx, JS_NewPlainObject(cx)};
174+
JS_GC(cx);
175+
JS::RootedObject nursery1{cx, JS_NewPlainObject(cx)};
176+
JS::RootedObject nursery2{cx, JS_NewPlainObject(cx)};
177+
178+
using ObjectMap = JS::GCHashMap<JS::Heap<JSObject*>, JS::Heap<JSObject*>,
179+
js::StableCellHasher<JS::Heap<JSObject*>>,
180+
js::SystemAllocPolicy>;
181+
using Cache = JS::WeakCache<ObjectMap>;
182+
Cache cache{JS::GetObjectZone(tenured1)};
183+
184+
cache.put(tenured1, tenured1);
185+
cache.put(tenured2, tenured2);
186+
cache.put(nursery1, nursery2); // These do not map to themselves.
187+
cache.put(nursery2, nursery1);
188+
cache.put(nullptr, nullptr); // nullptr entries should not be swept.
189+
190+
JS_GC(cx);
191+
CHECK(cache.has(tenured1));
192+
CHECK(cache.has(tenured2));
193+
CHECK(cache.has(nursery1));
194+
CHECK(cache.has(nursery2));
195+
CHECK(cache.has(nullptr));
196+
197+
tenured2 = nursery2 = nullptr;
198+
JS_GC(cx);
199+
CHECK(cache.has(tenured1));
200+
// nursery1 is gone because value is dead.
201+
CHECK(cache.has(nullptr));
202+
CHECK_EQUAL(cache.count(), unsigned{2});
203+
204+
return true;
205+
}
206+
END_TEST(testWeakCacheMapWithJSHeap)
207+
128208
// Exercise WeakCache<GCVector>.
129209
BEGIN_TEST(testWeakCacheGCVector) {
130210
// Create two objects tenured and two in the nursery. If zeal is on,
@@ -161,6 +241,45 @@ BEGIN_TEST(testWeakCacheGCVector) {
161241
}
162242
END_TEST(testWeakCacheGCVector)
163243

244+
BEGIN_TEST(testWeakCacheGCVectorWithJSHeap) {
245+
// Tests basically the same thing as testWeakCacheGCVector, but only using
246+
// APIs available to embedders.
247+
JS::RootedObject tenured1{cx, JS_NewPlainObject(cx)};
248+
JS::RootedObject tenured2{cx, JS_NewPlainObject(cx)};
249+
JS_GC(cx);
250+
JS::RootedObject nursery1{cx, JS_NewPlainObject(cx)};
251+
JS::RootedObject nursery2{cx, JS_NewPlainObject(cx)};
252+
253+
using ObjectVector = JS::WeakCache<
254+
JS::GCVector<JS::Heap<JSObject*>, 0, js::SystemAllocPolicy>>;
255+
ObjectVector cache{JS::GetObjectZone(tenured1)};
256+
257+
CHECK(cache.append(tenured1));
258+
CHECK(cache.append(tenured2));
259+
CHECK(cache.append(nursery1));
260+
CHECK(cache.append(nursery2));
261+
CHECK(cache.append(nullptr)); // nullptr entries should not be swept.
262+
CHECK_EQUAL(cache.get().length(), unsigned{5});
263+
264+
JS_GC(cx);
265+
CHECK_EQUAL(cache.get().length(), unsigned{5});
266+
CHECK_EQUAL(cache.get()[0], tenured1);
267+
CHECK_EQUAL(cache.get()[1], tenured2);
268+
CHECK_EQUAL(cache.get()[2], nursery1);
269+
CHECK_EQUAL(cache.get()[3], nursery2);
270+
CHECK_NULL(cache.get()[4].get());
271+
272+
tenured2 = nursery2 = nullptr;
273+
JS_GC(cx);
274+
CHECK_EQUAL(cache.get().length(), unsigned{3});
275+
CHECK_EQUAL(cache.get()[0], tenured1);
276+
CHECK_EQUAL(cache.get()[1], nursery1);
277+
CHECK_NULL(cache.get()[2].get());
278+
279+
return true;
280+
}
281+
END_TEST(testWeakCacheGCVectorWithJSHeap)
282+
164283
#ifdef JS_GC_ZEAL
165284

166285
// A simple structure that embeds an object pointer. We cripple the hash

0 commit comments

Comments
 (0)