Commit 7a22cf7a authored by Omer Katz's avatar Omer Katz Committed by Commit Bot

cppgc: Fix termination GCs

Whenever we destroy an object that contains a
(Weak)(CrossThread)Persistent, we call the Persistent's dtor which frees
the relevant PersistentNode. To get the PersistentRegion for the node,
we get the value, then get the the relevant page which holds a reference
to the heap which holds the regions.

During a termination GC there is no marking and no weak callback
processing. That means that, when destroying a Persistent, the page on
which the object referenced by the Persistent resides may have already
been swept and destroyed. Then when we try to get the page we cleared or
unallocated memory and crash.
This issue presented as a sweeper crash in the web tests and
content_browsertests.

This issue affects only Weak(CrossThread)Persistent since the region for
their strong counterparts are already cleared at the start of a
termination GC.
This is not an issue in the Blink impl because (1) Blink finds the
elevant regions through ThreadState without going through pages, and
(2) Blink runs a  full GC on termination that includes executing weak
callbacks.

Alternatively we could trace the Weak(CrossThread)Persistent region
which will run the weakness callbacks and clear all WeakPersistents.
The cost and outcome is equivalent.

Bug: chromium:1056170
Change-Id: I3db5b01424500eb695f9876247ef0c787d0d9ef2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2708645
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72897}
parent a3776a63
......@@ -106,6 +106,10 @@ void HeapBase::Terminate() {
// Clear root sets.
strong_persistent_region_.ClearAllUsedNodes();
strong_cross_thread_persistent_region_.ClearAllUsedNodes();
// Clear weak root sets, as the GC below does not execute weakness
// callbacks.
weak_persistent_region_.ClearAllUsedNodes();
weak_cross_thread_persistent_region_.ClearAllUsedNodes();
stats_collector()->NotifyMarkingStarted(
GarbageCollector::Config::CollectionType::kMajor,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment