Fix dead lock with transition array
The main thread can enter TransitionsAccessor::PutPrototypeTransition, lock the full_transition_array_access mutex, and then try to allocate, which can trigger a GC. It would then wait for all threads to be in a Safepoint. However, if a background thread tried to acquired a shared lock on full_transition_array_access (this happens from JsNativeContextSpecialization::ReducePropertyAccess, which eventually calls TransitionsAccessor::SearchTransition, which takes tries to take the shared lock) outside of a safepoint, we have a deadlock: the main thread waits for the background thread to be in a safepoint, but the background thread waits for the main thread to release the lock on full_transition_array_access. This CL fixes the issue by releasing the shared lock from the main thread during the "allocation" part of the growing of the transition array, so that the background thread can take the shared lock and then reach a safepoint; the main thread would then do the allocation and trigger the GC, and only then take the exclusive lock to replace the old transition array by the new one (but this wouldn't allocate). Bug: chromium:1393904 Change-Id: Ia3b94be575a0266c41c2fe6445090e551673b617 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4062039 Reviewed-by: Dominik Inführ <dinfuehr@chromium.org> Reviewed-by: Patrick Thier <pthier@chromium.org> Commit-Queue: Darius Mercadier <dmercadier@chromium.org> Cr-Commit-Position: refs/heads/main@{#84565}
This commit is contained in:
parent
d0b408a84a
commit
0c18a3a577
@ -371,13 +371,14 @@ Handle<WeakFixedArray> TransitionArray::GrowPrototypeTransitionArray(
|
||||
new_capacity = std::min({kMaxCachedPrototypeTransitions, new_capacity});
|
||||
DCHECK_GT(new_capacity, capacity);
|
||||
int grow_by = new_capacity - capacity;
|
||||
array = isolate->factory()->CopyWeakFixedArrayAndGrow(array, grow_by);
|
||||
Handle<WeakFixedArray> new_array =
|
||||
isolate->factory()->CopyWeakFixedArrayAndGrow(array, grow_by);
|
||||
if (capacity < 0) {
|
||||
// There was no prototype transitions array before, so the size
|
||||
// couldn't be copied. Initialize it explicitly.
|
||||
SetNumberOfPrototypeTransitions(*array, 0);
|
||||
SetNumberOfPrototypeTransitions(*new_array, 0);
|
||||
}
|
||||
return array;
|
||||
return new_array;
|
||||
}
|
||||
|
||||
// static
|
||||
@ -397,15 +398,33 @@ void TransitionsAccessor::PutPrototypeTransition(Isolate* isolate,
|
||||
int capacity = cache->length() - header;
|
||||
int transitions = TransitionArray::NumberOfPrototypeTransitions(*cache) + 1;
|
||||
|
||||
base::SharedMutexGuard<base::kExclusive> scope(
|
||||
isolate->full_transition_array_access());
|
||||
// We're not using a MutexGuard for {full_transition_array_access}, because
|
||||
// we'll need to release it before growing the transition array (if needed),
|
||||
// in order to avoid deadlock if a background thread is waiting for the shared
|
||||
// mutex outside of a safepoint. And after growing the array, we'll need to
|
||||
// re-lock it.
|
||||
base::SharedMutex* transition_array_mutex =
|
||||
isolate->full_transition_array_access();
|
||||
|
||||
transition_array_mutex->LockExclusive();
|
||||
if (transitions > capacity) {
|
||||
// Grow the array if compacting it doesn't free space.
|
||||
if (!TransitionArray::CompactPrototypeTransitionArray(isolate, *cache)) {
|
||||
transition_array_mutex->UnlockExclusive();
|
||||
if (capacity == TransitionArray::kMaxCachedPrototypeTransitions) return;
|
||||
|
||||
// GrowPrototypeTransitionArray can allocate, so it shouldn't hold the
|
||||
// exclusive lock on {full_transition_array_access} mutex, since
|
||||
// background threads could be waiting for the shared lock (outside of a
|
||||
// safe point). This is not an issue, because GrowPrototypeTransitionArray
|
||||
// doesn't actually modify in place the array, but instead return a new
|
||||
// array.
|
||||
transition_array_mutex->LockShared();
|
||||
cache = TransitionArray::GrowPrototypeTransitionArray(
|
||||
cache, 2 * transitions, isolate);
|
||||
transition_array_mutex->UnlockShared();
|
||||
|
||||
transition_array_mutex->LockExclusive();
|
||||
SetPrototypeTransitions(isolate, map, cache);
|
||||
}
|
||||
}
|
||||
@ -416,6 +435,8 @@ void TransitionsAccessor::PutPrototypeTransition(Isolate* isolate,
|
||||
|
||||
cache->Set(entry, HeapObjectReference::Weak(*target_map));
|
||||
TransitionArray::SetNumberOfPrototypeTransitions(*cache, last + 1);
|
||||
|
||||
transition_array_mutex->UnlockExclusive();
|
||||
}
|
||||
|
||||
// static
|
||||
|
Loading…
Reference in New Issue
Block a user