languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-06 14:59:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

[vm/concurrency] Add read lock asserts for Class direct_subclasses/direct_implementors

Browse source 
Issue https://github.com/dart-lang/sdk/issues/36097

Change-Id: Ie4f16c395efaf27f8a7b43af90e5eb703eb90ee5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170128
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This commit is contained in:
Alexander Aprelev 2020-11-06 20:05:53 +00:00 committed by commit-bot@chromium.org
parent 996626c089
commit 6200e47c2d
10 changed files with 40 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
runtime/vm/class_finalizer.cc
View file

@ -1386,6 +1386,7 @@ void ClassFinalizer::SortClasses() {
// Prevent background compiler from adding deferred classes or canonicalizing
// new types while classes are being sorted and type hashes are modified.
BackgroundCompiler::Stop(I);
SafepointWriteRwLocker ml(T, T->isolate_group()->program_lock());
ClassTable* table = I->class_table();
intptr_t num_cids = table->NumCids();

12
runtime/vm/compiler/backend/il.cc
View file

@ -269,10 +269,14 @@ void HierarchyInfo::BuildRangesForJIT(ClassTable* table,
Zone* zone = thread()->zone();
GrowableArray<intptr_t> cids;
SubclassFinder finder(zone, &cids, include_abstract);
if (use_subtype_test) {
finder.ScanImplementorClasses(dst_klass);
} else {
finder.ScanSubClasses(dst_klass);
{
SafepointReadRwLocker ml(thread(),
thread()->isolate_group()->program_lock());
if (use_subtype_test) {
finder.ScanImplementorClasses(dst_klass);
} else {
finder.ScanSubClasses(dst_klass);
}
}
// Sort all collected cids.

1
runtime/vm/compiler/call_specializer.cc
View file

@ -1515,6 +1515,7 @@ void TypedDataSpecializer::EnsureIsInitialized() {
auto& td_class = Class::Handle(Z);
auto& direct_implementors = GrowableObjectArray::Handle(Z);
SafepointReadRwLocker ml(thread_, thread_->isolate_group()->program_lock());
#define INIT_HANDLE(iface, member_name, type, cid) \
td_class = typed_data.LookupClass(Symbols::iface()); \

7
runtime/vm/compiler/cha.cc
View file

@ -43,6 +43,8 @@ bool CHA::HasSubclasses(const Class& cls) {
// Class Object has subclasses, although we do not keep track of them.
return true;
}
Thread* thread = Thread::Current();
SafepointReadRwLocker ml(thread, thread->isolate_group()->program_lock());
const GrowableObjectArray& direct_subclasses =
GrowableObjectArray::Handle(cls.direct_subclasses());
return !direct_subclasses.IsNull() && (direct_subclasses.Length() > 0);
@ -63,8 +65,11 @@ bool CHA::ConcreteSubclasses(const Class& cls,
class_ids->Add(cls.id());
}
// This is invoked from precompiler only, we can use unsafe version of
// Class::direct_subclasses getter.
ASSERT(FLAG_precompiled_mode);
const GrowableObjectArray& direct_subclasses =
GrowableObjectArray::Handle(cls.direct_subclasses());
GrowableObjectArray::Handle(cls.direct_subclasses_unsafe());
if (direct_subclasses.IsNull()) {
return true;
}

4
runtime/vm/compiler/jit/compiler.cc
View file

@ -620,6 +620,10 @@ CodePtr CompileParsedFunctionHelper::Compile(CompilationPipeline* pipeline) {
CheckIfBackgroundCompilerIsBeingStopped(optimized());
}
// Grab read program_lock outside of potential safepoint, that lock
// can't be waited for inside the safepoint.
SafepointReadRwLocker ml(thread(),
thread()->isolate_group()->program_lock());
// We have to ensure no mutators are running, because:
//
// a) We allocate an instructions object, which might cause us to

1
runtime/vm/heap/safepoint.h
View file

@ -233,6 +233,7 @@ class TransitionGeneratedToNative : public TransitionSafepointState {
class TransitionVMToBlocked : public TransitionSafepointState {
public:
explicit TransitionVMToBlocked(Thread* T) : TransitionSafepointState(T) {
ASSERT(!T->isolate_group()->safepoint_handler()->IsOwnedByTheThread(T));
// A thread blocked on a monitor is considered to be at a safepoint.
ASSERT(T->execution_state() == Thread::kThreadInVM);
T->set_execution_state(Thread::kThreadInBlockedState);

2
runtime/vm/isolate.cc
View file

@ -426,7 +426,7 @@ void IsolateGroup::RegisterIsolateLocked(Isolate* isolate) {
}
bool IsolateGroup::ContainsOnlyOneIsolate() {
SafepointWriteRwLocker ml(Thread::Current(), isolates_lock_.get());
SafepointReadRwLocker ml(Thread::Current(), isolates_lock_.get());
return isolate_count_ == 0;
}

20
runtime/vm/isolate_reload_test.cc
View file

@ -2364,7 +2364,7 @@ ISOLATE_UNIT_TEST_CASE(IsolateReload_DirectSubclasses_Success) {
// Keep track of how many subclasses an Stopwatch has.
auto& subclasses =
GrowableObjectArray::Handle(stopwatch_cls.direct_subclasses());
GrowableObjectArray::Handle(stopwatch_cls.direct_subclasses_unsafe());
intptr_t saved_subclass_count = subclasses.IsNull() ? 0 : subclasses.Length();
const char* kScript =
@ -2383,7 +2383,7 @@ ISOLATE_UNIT_TEST_CASE(IsolateReload_DirectSubclasses_Success) {
}
// Stopwatch has one non-core subclass.
subclasses = stopwatch_cls.direct_subclasses();
subclasses = stopwatch_cls.direct_subclasses_unsafe();
EXPECT_EQ(saved_subclass_count + 1, subclasses.Length());
// The new subclass is named AStopwatch.
@ -2410,7 +2410,7 @@ ISOLATE_UNIT_TEST_CASE(IsolateReload_DirectSubclasses_Success) {
}
// Stopwatch still has only one non-core subclass (AStopwatch is gone).
subclasses = stopwatch_cls.direct_subclasses();
subclasses = stopwatch_cls.direct_subclasses_unsafe();
EXPECT_EQ(saved_subclass_count + 1, subclasses.Length());
// The new subclass is named BStopwatch.
@ -2431,7 +2431,7 @@ ISOLATE_UNIT_TEST_CASE(IsolateReload_DirectSubclasses_GhostSubclass) {
// Keep track of how many subclasses an Stopwatch has.
auto& subclasses =
GrowableObjectArray::Handle(stopwatch_cls.direct_subclasses());
GrowableObjectArray::Handle(stopwatch_cls.direct_subclasses_unsafe());
intptr_t saved_subclass_count = subclasses.IsNull() ? 0 : subclasses.Length();
const char* kScript =
@ -2450,7 +2450,7 @@ ISOLATE_UNIT_TEST_CASE(IsolateReload_DirectSubclasses_GhostSubclass) {
}
// Stopwatch has one new subclass.
subclasses = stopwatch_cls.direct_subclasses();
subclasses = stopwatch_cls.direct_subclasses_unsafe();
EXPECT_EQ(saved_subclass_count + 1, subclasses.Length());
// The new subclass is named AStopwatch.
@ -2474,7 +2474,7 @@ ISOLATE_UNIT_TEST_CASE(IsolateReload_DirectSubclasses_GhostSubclass) {
}
// Stopwatch has two non-core subclasses.
subclasses = stopwatch_cls.direct_subclasses();
subclasses = stopwatch_cls.direct_subclasses_unsafe();
EXPECT_EQ(saved_subclass_count + 2, subclasses.Length());
// The non-core subclasses are AStopwatch and BStopwatch.
@ -2500,7 +2500,7 @@ ISOLATE_UNIT_TEST_CASE(IsolateReload_DirectSubclasses_Failure) {
// Keep track of how many subclasses an Stopwatch has.
auto& subclasses =
GrowableObjectArray::Handle(stopwatch_cls.direct_subclasses());
GrowableObjectArray::Handle(stopwatch_cls.direct_subclasses_unsafe());
intptr_t saved_subclass_count = subclasses.IsNull() ? 0 : subclasses.Length();
const char* kScript =
@ -2524,11 +2524,11 @@ ISOLATE_UNIT_TEST_CASE(IsolateReload_DirectSubclasses_Failure) {
}
// Stopwatch has one non-core subclass...
subclasses = stopwatch_cls.direct_subclasses();
subclasses = stopwatch_cls.direct_subclasses_unsafe();
EXPECT_EQ(saved_subclass_count + 1, subclasses.Length());
// ... and the non-core subclass is named AStopwatch.
subclasses = stopwatch_cls.direct_subclasses();
subclasses = stopwatch_cls.direct_subclasses_unsafe();
new_subclass = subclasses.At(subclasses.Length() - 1);
name = Class::Cast(new_subclass).Name();
EXPECT_STREQ("AStopwatch", name.ToCString());
@ -2556,7 +2556,7 @@ ISOLATE_UNIT_TEST_CASE(IsolateReload_DirectSubclasses_Failure) {
// If we don't clean up the subclasses, we would find BStopwatch in
// the list of subclasses, which would be bad. Make sure that
// Stopwatch still has only one non-core subclass...
subclasses = stopwatch_cls.direct_subclasses();
subclasses = stopwatch_cls.direct_subclasses_unsafe();
EXPECT_EQ(saved_subclass_count + 1, subclasses.Length());
// ...and the non-core subclass is still named AStopwatch.

7
runtime/vm/object.h
View file

@ -1194,6 +1194,8 @@ class Class : public Object {
// Returns the list of classes directly implementing this class.
GrowableObjectArrayPtr direct_implementors() const {
DEBUG_ASSERT(
IsolateGroup::Current()->program_lock()->IsCurrentThreadReader());
return raw_ptr()->direct_implementors_;
}
void AddDirectImplementor(const Class& subclass, bool is_mixin) const;
@ -1201,6 +1203,11 @@ class Class : public Object {
// Returns the list of classes having this class as direct superclass.
GrowableObjectArrayPtr direct_subclasses() const {
DEBUG_ASSERT(
IsolateGroup::Current()->program_lock()->IsCurrentThreadReader());
return direct_subclasses_unsafe();
}
GrowableObjectArrayPtr direct_subclasses_unsafe() const {
return raw_ptr()->direct_subclasses_;
}
void AddDirectSubclass(const Class& subclass) const;

2
runtime/vm/object_service.cc
View file

@ -154,7 +154,7 @@ void Class::PrintJSONImpl(JSONStream* stream, bool ref) const {
{
JSONArray subclasses_array(&jsobj, "subclasses");
const GrowableObjectArray& subclasses =
GrowableObjectArray::Handle(direct_subclasses());
GrowableObjectArray::Handle(direct_subclasses_unsafe());
if (!subclasses.IsNull()) {
Class& subclass = Class::Handle();
for (intptr_t i = 0; i < subclasses.Length(); ++i) {