[vm/concurrency] Move no-reload-scope counter to Thread

Whether a given isolate allows reloading is really a property of the
thread not the isolate (where it was before) or isolate group (where
it was moved to), so this CL moves it to Thread.

As the thread's stack is going down and up, NoReloadScope's might be
entered or exited, just as with our many other scopes, such as
NoOOBMessageScope/...

Longer term we might need to find a better mechanism to deal with
different OOB messages. For example: Reload as well as OOB messages
are disabled during static field initialization. Though that means *all*
OOB messages cannot reach the isolate during initialization of a static
field (which can do arbitrary amount of work).

This is part of making hot-reload work with isolate groups.

Issue #36097

TEST=Refactoring of existing code.

Change-Id: I9e80c6d4a184c54c0373fef13af3264cc3f27ece
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/187002
Reviewed-by: Alexander Aprelev <[email protected]>
Reviewed-by: Ryan Macnak <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>

Author: mkustermann

runtime/lib/mirrors.cc

@@ -666,7 +666,7 @@ DEFINE_NATIVE_ENTRY(IsolateMirror_loadUri, 0, 1) {
     ThrowLanguageError("no library handler registered");
   }
 
-  NoReloadScope no_reload(isolate->group(), thread);
+  NoReloadScope no_reload(thread);
 
   // Canonicalize library URI.
   String& canonical_uri = String::Handle(zone);

runtime/vm/exceptions.cc

@@ -1191,7 +1191,7 @@ ObjectPtr Exceptions::Create(ExceptionType type, const Array& arguments) {
   }
 
   Thread* thread = Thread::Current();
-  NoReloadScope no_reload_scope(thread->isolate_group(), thread);
+  NoReloadScope no_reload_scope(thread);
   return DartLibraryCalls::InstanceCreate(library, *class_name,
                                           *constructor_name, arguments);
 }

runtime/vm/isolate.cc

@@ -866,22 +866,6 @@ NoOOBMessageScope::~NoOOBMessageScope() {
   }
 }
 
-NoReloadScope::NoReloadScope(IsolateGroup* isolate_group, Thread* thread)
-    : ThreadStackResource(thread), isolate_group_(isolate_group) {
-#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
-  ASSERT(isolate_group_ != NULL);
-  isolate_group_->no_reload_scope_depth_.fetch_add(1);
-  ASSERT(isolate_group_->no_reload_scope_depth_ >= 0);
-#endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
-}
-
-NoReloadScope::~NoReloadScope() {
-#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
-  isolate_group_->no_reload_scope_depth_.fetch_sub(1);
-  ASSERT(isolate_group_->no_reload_scope_depth_ >= 0);
-#endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
-}
-
 Bequest::~Bequest() {
   IsolateGroup* isolate_group = IsolateGroup::Current();
   CHECK_ISOLATE_GROUP(isolate_group);
@@ -1500,7 +1484,7 @@ MessageHandler::MessageStatus IsolateMessageHandler::ProcessUnhandledException(
         T->isolate()->name(), result.ToErrorCString());
   }
 
-  NoReloadScope no_reload_scope(T->isolate_group(), T);
+  NoReloadScope no_reload(T);
   // Generate the error and stacktrace strings for the error message.
   const char* exception_cstr = nullptr;
   const char* stacktrace_cstr = nullptr;
@@ -2002,13 +1986,14 @@ void Isolate::BuildName(const char* name_prefix) {
 
 #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
 bool IsolateGroup::CanReload() {
+  auto thread = Thread::Current();
   // TODO(dartbug.com/36097): As part of implementing hot-reload support for
   // --enable-isolate-groups, we should make the reload implementation wait for
   // any outstanding isolates to "check-in" (which should only be done after
   // making them runnable) instead of refusing the reload.
   bool all_runnable = true;
   {
-    SafepointReadRwLocker ml(Thread::Current(), isolates_lock_.get());
+    SafepointReadRwLocker ml(thread, isolates_lock_.get());
     for (Isolate* isolate : isolates_) {
       if (!isolate->is_runnable()) {
         all_runnable = false;
@@ -2017,7 +2002,7 @@ bool IsolateGroup::CanReload() {
     }
   }
   return !IsolateGroup::IsSystemIsolateGroup(this) && all_runnable &&
-         !IsReloading() && (no_reload_scope_depth_ == 0) &&
+         !IsReloading() && (thread->no_reload_scope_depth_ == 0) &&
          Isolate::IsolateCreationEnabled() &&
          OSThread::Current()->HasStackHeadroom(64 * KB);
 }

runtime/vm/isolate.h

@@ -154,17 +154,6 @@ class NoOOBMessageScope : public ThreadStackResource {
   DISALLOW_COPY_AND_ASSIGN(NoOOBMessageScope);
 };
 
-// Disallow isolate reload.
-class NoReloadScope : public ThreadStackResource {
- public:
-  NoReloadScope(IsolateGroup* isolate_group, Thread* thread);
-  ~NoReloadScope();
-
- private:
-  IsolateGroup* isolate_group_;
-  DISALLOW_COPY_AND_ASSIGN(NoReloadScope);
-};
-
 // Fixed cache for exception handler lookup.
 typedef FixedCache<intptr_t, ExceptionHandlerInfo, 16> HandlerInfoCache;
 // Fixed cache for catch entry state lookup.
@@ -826,7 +815,6 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
   friend class StackFrame;  // For `[isolates_].First()`.
   // For `object_store_shared_untag()`, `class_table_shared_untag()`
   friend class Isolate;
-  friend class NoReloadScope;  // no_reload_scope_depth_
 
 #define ISOLATE_GROUP_FLAG_BITS(V)                                             \
   V(AllClassesFinalized)                                                       \
@@ -882,8 +870,6 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
 #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
   int64_t last_reload_timestamp_;
   std::shared_ptr<IsolateGroupReloadContext> group_reload_context_;
-  RelaxedAtomic<intptr_t> no_reload_scope_depth_ =
-      0;  // we can only reload when this is 0.
   // Per-isolate-group copy of FLAG_reload_every.
   RelaxedAtomic<intptr_t> reload_every_n_stack_overflow_checks_;
   ProgramReloadContext* program_reload_context_ = nullptr;

runtime/vm/kernel_loader.cc

@@ -500,7 +500,7 @@ void KernelLoader::EvaluateDelayedPragmas() {
 
   Thread* thread = Thread::Current();
   NoOOBMessageScope no_msg_scope(thread);
-  NoReloadScope no_reload_scope(thread->isolate_group(), thread);
+  NoReloadScope no_reload_scope(thread);
 
   Function& function = Function::Handle();
   Library& library = Library::Handle();
@@ -2096,7 +2096,7 @@ void KernelLoader::LoadProcedure(const Library& library,
     } else {
       Thread* thread = Thread::Current();
       NoOOBMessageScope no_msg_scope(thread);
-      NoReloadScope no_reload_scope(thread->isolate_group(), thread);
+      NoReloadScope no_reload_scope(thread);
       library.GetMetadata(function);
     }
   }

runtime/vm/object.cc

@@ -10842,7 +10842,7 @@ ObjectPtr Field::EvaluateInitializer() const {
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
   NoOOBMessageScope no_msg_scope(thread);
-  NoReloadScope no_reload_scope(thread->isolate_group(), thread);
+  NoReloadScope no_reload_scope(thread);
   const Function& initializer = Function::Handle(EnsureInitializerFunction());
   return DartEntry::InvokeFunction(initializer, Object::empty_array());
 }
@@ -18063,7 +18063,7 @@ StringPtr LanguageError::FormatMessage() const {
 
 const char* LanguageError::ToErrorCString() const {
   Thread* thread = Thread::Current();
-  NoReloadScope no_reload_scope(thread->isolate_group(), thread);
+  NoReloadScope no_reload_scope(thread);
   const String& msg_str = String::Handle(FormatMessage());
   return msg_str.ToCString();
 }
@@ -18113,7 +18113,7 @@ void UnhandledException::set_stacktrace(const Instance& stacktrace) const {
 const char* UnhandledException::ToErrorCString() const {
   Thread* thread = Thread::Current();
   auto isolate_group = thread->isolate_group();
-  NoReloadScope no_reload_scope(isolate_group, thread);
+  NoReloadScope no_reload_scope(thread);
   HANDLESCOPE(thread);
   Object& strtmp = Object::Handle();
   const char* exc_str;

runtime/vm/runtime_entry.cc

@@ -2552,7 +2552,7 @@ static void HandleStackOverflowTestCases(Thread* thread) {
 
     const char* script_uri;
     {
-      NoReloadScope no_reload(isolate_group, thread);
+      NoReloadScope no_reload(thread);
       const Library& lib =
           Library::Handle(isolate_group->object_store()->_internal_library());
       const Class& cls = Class::Handle(

runtime/vm/thread.h

@@ -565,6 +565,8 @@ class Thread : public ThreadState {
 #endif
   }
 
+  bool IsInNoReloadScope() const { return no_reload_scope_depth_ > 0; }
+
 #define DEFINE_OFFSET_METHOD(type_name, member_name, expr, default_init_value) \
   static intptr_t member_name##offset() {                                      \
     return OFFSET_OF(Thread, member_name);                                     \
@@ -1000,6 +1002,7 @@ class Thread : public ThreadState {
   mutable Monitor thread_lock_;
   ApiLocalScope* api_reusable_scope_;
   int32_t no_callback_scope_depth_;
+  intptr_t no_reload_scope_depth_ = 0;
 #if defined(DEBUG)
   int32_t no_safepoint_scope_depth_;
 #endif
@@ -1088,6 +1091,7 @@ class Thread : public ThreadState {
   friend class IsolateGroup;
   friend class IsolateTestHelper;
   friend class NoOOBMessageScope;
+  friend class NoReloadScope;
   friend class Simulator;
   friend class StackZone;
   friend class ThreadRegistry;
@@ -1137,6 +1141,28 @@ class NoSafepointScope : public ValueObject {
 };
 #endif  // defined(DEBUG)
 
+class NoReloadScope : public ThreadStackResource {
+ public:
+  explicit NoReloadScope(Thread* thread)
+      : ThreadStackResource(thread), thread_(thread) {
+#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
+    thread->no_reload_scope_depth_++;
+    ASSERT(thread->no_reload_scope_depth_ >= 0);
+#endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
+  }
+
+  ~NoReloadScope() {
+#if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
+    thread_->no_reload_scope_depth_ -= 1;
+    ASSERT(thread_->no_reload_scope_depth_ >= 0);
+#endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
+  }
+
+ private:
+  Thread* thread_;
+  DISALLOW_COPY_AND_ASSIGN(NoReloadScope);
+};
+
 // Within a EnterCompilerScope, the thread must operate on cloned fields.
 #if defined(DEBUG)
 class EnterCompilerScope : public ThreadStackResource {