commit cdef81484a9c62b2a5687abfc7ff38c7792117f7
parent 3c4c4ac0ef31b40093c3b2d02dae2db2c5ec4f7d
Author: Cristian Tuns <ctuns@mozilla.com>
Date: Mon, 10 Nov 2025 11:37:10 -0500
Revert "Bug 1997894 - Fix delayed check in AssertCellIsNotGray to work for symbols r=jandem" for causing multiple assertion failures in GC.cpp
This reverts commit 76fab0dd61c9b489203d808d2fb9a603164026e0.
Diffstat:
4 files changed, 21 insertions(+), 25 deletions(-)
diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp
@@ -3116,8 +3116,6 @@ void GCRuntime::beginMarkPhase(AutoGCSession& session) {
{
BufferAllocator::MaybeLock lock;
for (GCZonesIter zone(this); !zone.done(); zone.next()) {
- MOZ_ASSERT(zone->cellsToAssertNotGray().empty());
-
// In an incremental GC, clear the arena free lists to ensure that
// subsequent allocations refill them and end up marking new cells black.
// See arenaAllocatedDuringGC().
@@ -3578,11 +3576,11 @@ void GCRuntime::checkGCStateNotInUse() {
MOZ_ASSERT(!zone->isOnList());
MOZ_ASSERT(!zone->gcNextGraphNode);
MOZ_ASSERT(!zone->gcNextGraphComponent);
- MOZ_ASSERT(zone->cellsToAssertNotGray().empty());
zone->bufferAllocator.checkGCStateNotInUse();
}
MOZ_ASSERT(zonesToMaybeCompact.ref().isEmpty());
+ MOZ_ASSERT(cellsToAssertNotGray.ref().empty());
MOZ_ASSERT(!atomsUsedByUncollectedZones.ref());
@@ -5555,15 +5553,15 @@ JS_PUBLIC_API void js::gc::detail::AssertCellIsNotGray(const Cell* cell) {
// called during GC and while iterating the heap for memory reporting.
MOZ_ASSERT(!JS::RuntimeHeapIsCycleCollecting());
- Zone* zone = tc->zone();
- if (zone->isGCMarkingBlackAndGray()) {
+ if (tc->zone()->isGCMarkingBlackAndGray()) {
// We are doing gray marking in the cell's zone. Even if the cell is
// currently marked gray it may eventually be marked black. Delay checking
// non-black cells until we finish gray marking.
if (!tc->isMarkedBlack()) {
+ JSRuntime* rt = tc->zone()->runtimeFromMainThread();
AutoEnterOOMUnsafeRegion oomUnsafe;
- if (!zone->cellsToAssertNotGray().append(cell)) {
+ if (!rt->gc.cellsToAssertNotGray.ref().append(cell)) {
oomUnsafe.crash("Can't append to delayed gray checks list");
}
}
diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h
@@ -1302,6 +1302,13 @@ class GCRuntime {
/* The test marking queue might want to be marking a particular color. */
mozilla::Maybe<js::gc::MarkColor> queueMarkColor;
+
+ // During gray marking, delay AssertCellIsNotGray checks by
+ // recording the cell pointers here and checking after marking has
+ // finished.
+ MainThreadData<Vector<const Cell*, 0, SystemAllocPolicy>>
+ cellsToAssertNotGray;
+ friend void js::gc::detail::AssertCellIsNotGray(const Cell*);
#endif
friend class SweepGroupsIter;
diff --git a/js/src/gc/Sweeping.cpp b/js/src/gc/Sweeping.cpp
@@ -1224,6 +1224,7 @@ IncrementalProgress GCRuntime::beginMarkingSweepGroup(JS::GCContext* gcx,
for (auto& marker : markers) {
MOZ_ASSERT(marker->markColor() == MarkColor::Black);
}
+ MOZ_ASSERT(cellsToAssertNotGray.ref().empty());
#endif
gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::MARK);
@@ -1233,8 +1234,6 @@ IncrementalProgress GCRuntime::beginMarkingSweepGroup(JS::GCContext* gcx,
// will be marked through, as they are not marked with
// TraceCrossCompartmentEdge.
for (SweepGroupZonesIter zone(this); !zone.done(); zone.next()) {
- MOZ_ASSERT_IF(!zone->isGCMarkingBlackAndGray(),
- zone->cellsToAssertNotGray().empty());
zone->changeGCState(zone->initialMarkingState(), Zone::MarkBlackAndGray);
}
@@ -1654,14 +1653,14 @@ IncrementalProgress GCRuntime::beginSweepingSweepGroup(JS::GCContext* gcx,
if (zone->isAtomsZone()) {
sweepingAtoms = true;
}
+ }
#ifdef DEBUG
- for (const auto* cell : zone->cellsToAssertNotGray()) {
- JS::AssertCellIsNotGray(cell);
- }
- zone->cellsToAssertNotGray().clearAndFree();
-#endif
+ for (const auto* cell : cellsToAssertNotGray.ref()) {
+ JS::AssertCellIsNotGray(cell);
}
+ cellsToAssertNotGray.ref().clearAndFree();
+#endif
// Updating the atom marking bitmaps. This marks atoms referenced by
// uncollected zones so cannot be done in parallel with the other sweeping
diff --git a/js/src/gc/Zone.h b/js/src/gc/Zone.h
@@ -451,6 +451,10 @@ class Zone : public js::ZoneAllocator, public js::gc::GraphNodeBase<JS::Zone> {
js::MainThreadData<js::StringStats> previousGCStringStats;
js::MainThreadData<js::StringStats> stringStats;
+#ifdef DEBUG
+ js::MainThreadData<unsigned> gcSweepGroupIndex;
+#endif
+
js::gc::PretenuringZone pretenuring;
private:
@@ -550,16 +554,6 @@ class Zone : public js::ZoneAllocator, public js::gc::GraphNodeBase<JS::Zone> {
using ObjectVector = js::GCVector<JSObject*, 0, js::SystemAllocPolicy>;
js::MainThreadOrGCTaskData<ObjectVector> objectsWithWeakPointers;
-#ifdef DEBUG
- js::MainThreadData<unsigned> gcSweepGroupIndex;
-
- // During gray marking, delay AssertCellIsNotGray checks by
- // recording the cell pointers here and checking after marking has
- // finished.
- js::MainThreadData<js::Vector<const js::gc::Cell*, 0, js::SystemAllocPolicy>>
- cellsToAssertNotGray_;
-#endif
-
public:
#ifdef JS_GC_ZEAL
// Must come after weakCaches_ above.
@@ -962,8 +956,6 @@ class Zone : public js::ZoneAllocator, public js::gc::GraphNodeBase<JS::Zone> {
// For testing purposes, return the index of the sweep group which this zone
// was swept in in the last GC.
unsigned lastSweepGroupIndex() { return gcSweepGroupIndex; }
-
- auto& cellsToAssertNotGray() { return cellsToAssertNotGray_.ref(); }
#endif
// Support for invalidating fuses
