[vm/concurrency] Ensure static field registration is done under programlock

Also prepare for being able to grow field table of other isolates when
registering a new static field.

Issue dart-lang/sdk#36097

TEST=Tests using --enable-isolate-groups.

Change-Id: If1bc272e71da5cc89f962cfcac04d72c9314cf51
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175066
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>

Author: mkustermann

runtime/vm/compiler/backend/type_propagator_test.cc

@@ -187,7 +187,11 @@ ISOLATE_UNIT_TEST_CASE(TypePropagator_Refinement) {
                  /*is_reflectable=*/true,
                  /*is_late=*/false, object_class, Object::dynamic_type(),
                  TokenPosition::kNoSource, TokenPosition::kNoSource));
-  thread->isolate_group()->RegisterStaticField(field, Instance::Handle());
+  {
+    SafepointWriteRwLocker locker(thread,
+                                  thread->isolate_group()->program_lock());
+    thread->isolate_group()->RegisterStaticField(field, Instance::Handle());
+  }
 
   FlowGraphBuilderHelper H;
 

runtime/vm/dart.cc

@@ -697,8 +697,7 @@ static bool CloneIntoChildIsolateAOT(Thread* T,
   }
   I->isolate_object_store()->Init();
   I->isolate_object_store()->PreallocateObjects();
-  I->set_field_table(T, source_isolate_group->initial_field_table()->Clone(
-                            /*is_isolate_field_table=*/true));
+  I->set_field_table(T, source_isolate_group->initial_field_table()->Clone(I));
 
   return true;
 }
@@ -743,8 +742,7 @@ ErrorPtr Dart::InitIsolateFromSnapshot(Thread* T,
       return error.raw();
     }
 
-    I->set_field_table(T, I->group()->initial_field_table()->Clone(
-                              /*is_isolate_field_table=*/true));
+    I->set_field_table(T, I->group()->initial_field_table()->Clone(I));
 
 #if defined(SUPPORT_TIMELINE)
     if (tbes.enabled()) {

runtime/vm/field_table.cc

@@ -98,13 +98,13 @@ void FieldTable::Grow(intptr_t new_capacity) {
   // via store to table_.
   std::atomic_thread_fence(std::memory_order_release);
   table_ = new_table;
-  if (is_isolate_field_table_) {
-    Thread::Current()->field_table_values_ = table_;
+  if (isolate_ != nullptr) {
+    isolate_->mutator_thread()->field_table_values_ = table_;
   }
 }
 
-FieldTable* FieldTable::Clone(bool is_isolate_field_table) {
-  FieldTable* clone = new FieldTable(is_isolate_field_table);
+FieldTable* FieldTable::Clone(Isolate* for_isolate) {
+  FieldTable* clone = new FieldTable(for_isolate);
   auto new_table = static_cast<InstancePtr*>(
       malloc(capacity_ * sizeof(InstancePtr)));  // NOLINT
   memmove(new_table, table_, capacity_ * sizeof(InstancePtr));

runtime/vm/field_table.h

@@ -16,17 +16,18 @@
 
 namespace dart {
 
+class Isolate;
 class Field;
 
 class FieldTable {
  public:
-  explicit FieldTable(bool is_isolate_field_table)
+  explicit FieldTable(Isolate* isolate)
       : top_(0),
         capacity_(0),
         free_head_(-1),
         table_(nullptr),
         old_tables_(new MallocGrowableArray<InstancePtr*>()),
-        is_isolate_field_table_(is_isolate_field_table) {}
+        isolate_(isolate) {}
 
   ~FieldTable();
 
@@ -56,7 +57,7 @@ class FieldTable {
   }
   void SetAt(intptr_t index, InstancePtr raw_instance);
 
-  FieldTable* Clone(bool is_isolate_field_table);
+  FieldTable* Clone(Isolate* for_isolate);
 
   void VisitObjectPointers(ObjectPointerVisitor* visitor);
 
@@ -83,10 +84,10 @@ class FieldTable {
   // so it will get freed when its are no longer in use.
   MallocGrowableArray<InstancePtr*>* old_tables_;
 
-  // Whether this table is used as a isolate-specific table of it's global field
-  // values and therefore needs to keep the cached field table in the `Thread`
-  // object up-to-date.
-  bool is_isolate_field_table_;
+  // If non-NULL, it will specify the isolate this field table belongs to.
+  // Growing the field table will keep the cached field table on the isolate's
+  // mutator thread up-to-date.
+  Isolate* isolate_;
 
   DISALLOW_COPY_AND_ASSIGN(FieldTable);
 };

runtime/vm/isolate.cc

@@ -353,7 +353,7 @@ IsolateGroup::IsolateGroup(std::shared_ptr<IsolateGroupSource> source,
       store_buffer_(new StoreBuffer()),
       heap_(nullptr),
       saved_unlinked_calls_(Array::null()),
-      initial_field_table_(new FieldTable(/*is_isolate_field_table=*/false)),
+      initial_field_table_(new FieldTable(/*isolate=*/nullptr)),
       symbols_lock_(new SafepointRwLock()),
       type_canonicalization_mutex_(
           NOT_IN_PRODUCT("IsolateGroup::type_canonicalization_mutex_")),
@@ -907,6 +907,8 @@ void Isolate::ValidateClassTable() {
 
 void IsolateGroup::RegisterStaticField(const Field& field,
                                        const Instance& initial_value) {
+  ASSERT(program_lock()->IsCurrentThreadWriter());
+
   ASSERT(field.is_static());
   initial_field_table()->Register(field);
   initial_field_table()->SetAt(field.field_id(), initial_value.raw());
@@ -1631,7 +1633,7 @@ Isolate::Isolate(IsolateGroup* isolate_group,
       default_tag_(UserTag::null()),
       ic_miss_code_(Code::null()),
       shared_class_table_(isolate_group->shared_class_table()),
-      field_table_(new FieldTable(/*is_isolate_field_table=*/true)),
+      field_table_(new FieldTable(/*isolate=*/this)),
       isolate_group_(isolate_group),
       isolate_object_store_(
           new IsolateObjectStore(isolate_group->object_store())),

runtime/vm/object_test.cc

@@ -3065,7 +3065,11 @@ static FieldPtr CreateTestField(const char* name) {
   const Field& field = Field::Handle(Field::New(
       field_name, true, false, false, true, false, cls, Object::dynamic_type(),
       TokenPosition::kMinSource, TokenPosition::kMinSource));
-  thread->isolate_group()->RegisterStaticField(field, Instance::sentinel());
+  {
+    SafepointWriteRwLocker locker(thread,
+                                  thread->isolate_group()->program_lock());
+    thread->isolate_group()->RegisterStaticField(field, Instance::sentinel());
+  }
   return field.raw();
 }