[vm] Relax assertion: Allow nested acquires of locks after acquiring safepoint operation

If a thread owns a lock already before acquiring the safepoint
operation, it can re-acquire the same lock inside the safepoint
operation (as it's a NOP and canoot lead to deadlocks).

TEST=ci

Change-Id: I4a7c3526683e09d4408d97aca9e9b59d6ca53d19
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/318662
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
This commit is contained in:
Martin Kustermann 2023-08-08 10:17:36 +00:00
parent b6409c0f04
commit 86adabe0ef
2 changed files with 23 additions and 3 deletions

View file

@ -6435,7 +6435,6 @@ class ProgramDeserializationRoots : public DeserializationRoots {
void PostLoad(Deserializer* d, const Array& refs) {
auto isolate_group = d->isolate_group();
{
SafepointWriteRwLocker ml(d->thread(), isolate_group->program_lock());
isolate_group->class_table()->CopySizesFromClassObjects();
}
d->heap()->old_space()->EvaluateAfterLoading();
@ -8762,7 +8761,6 @@ void Deserializer::Deserialize(DeserializationRoots* roots) {
{
TIMELINE_DURATION(thread(), Isolate, "ReadFill");
SafepointWriteRwLocker ml(thread(), isolate_group()->program_lock());
for (intptr_t i = 0; i < num_clusters_; i++) {
clusters_[i]->ReadFill(this, primary);
#if defined(DEBUG)
@ -9166,6 +9164,11 @@ ApiErrorPtr FullSnapshotReader::ReadVMSnapshot() {
return ConvertToApiError(error);
}
// Even though there's no concurrent threads we have to guard agains, some
// logic we do in deserialization triggers common code that asserts the
// program lock is held.
SafepointWriteRwLocker ml(thread_, isolate_group()->program_lock());
Deserializer deserializer(thread_, kind_, buffer_, size_, data_image_,
instructions_image_, /*is_non_root_unit=*/false,
offset);
@ -9207,6 +9210,11 @@ ApiErrorPtr FullSnapshotReader::ReadProgramSnapshot() {
return ConvertToApiError(error);
}
// Even though there's no concurrent threads we have to guard agains, some
// logic we do in deserialization triggers common code that asserts the
// program lock is held.
SafepointWriteRwLocker ml(thread_, isolate_group()->program_lock());
Deserializer deserializer(thread_, kind_, buffer_, size_, data_image_,
instructions_image_, /*is_non_root_unit=*/false,
offset);

View file

@ -109,7 +109,11 @@ bool SafepointRwLock::EnterRead() {
auto thread = Thread::Current();
// Attempt to acquire a lock while owning a safepoint could lead to a deadlock
// (some other thread might be forced to a safepoint while holding this lock).
ASSERT(thread == nullptr || thread->CanAcquireSafepointLocks());
//
// Though if the lock was already acquired by this thread before entering a
// safepoint, we do allow the nested acquire (which is a NOP).
DEBUG_ASSERT(thread == nullptr || thread->CanAcquireSafepointLocks() ||
IsCurrentThreadReader());
const bool can_block_without_safepoint = thread == nullptr;
@ -169,6 +173,14 @@ void SafepointRwLock::LeaveRead() {
void SafepointRwLock::EnterWrite() {
// No need to safepoint if the current thread is not attached.
auto thread = Thread::Current();
// Attempt to acquire a lock while owning a safepoint could lead to a deadlock
// (some other thread might be forced to a safepoint while holding this lock).
//
// Though if the lock was already acquired by this thread before entering a
// safepoint, we do allow the nested acquire (which is a NOP).
DEBUG_ASSERT(thread == nullptr || thread->CanAcquireSafepointLocks() ||
IsCurrentThreadWriter());
const bool can_block_without_safepoint = thread == nullptr;
if (!TryEnterWrite(can_block_without_safepoint)) {