[ VM ] Inherit system isolate property on child isolate spawn

This prevents --pause-isolates-on-start and --pause-isolates-on-exit
from impacting isolates spawned from system isolates. This should be
revisited to determine if:

- Inheriting the system isolate property is actually what we want to do
  in general
- Isolate.spawn* APIs should instead include a parameter to specify an
  isolate as a system isolate

Fixes https://github.com/dart-lang/sdk/issues/54729

TEST=Manual testing

Change-Id: Ibacdea845db6344148c11bebf001e4cac3377a62
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/348460
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>

pkg/_fe_analyzer_shared/lib/src/macros/executor/isolated_executor.dart

@@ -50,7 +50,8 @@ class _SingleIsolatedMacroExecutor extends ExternalMacroExecutorBase {
     ReceivePort receivePort = new ReceivePort();
     Isolate isolate = await Isolate.spawnUri(
         uriToSpawn, arguments, receivePort.sendPort,
-        packageConfig: packageConfig);
+        packageConfig: packageConfig,
+        debugName: 'macro-executor ($uriToSpawn)');
     Completer<SendPort> sendPortCompleter = new Completer();
     StreamController<Object> messageStreamController =
         new StreamController(sync: true);

runtime/lib/isolate.cc

@@ -595,7 +595,24 @@ class SpawnIsolateTask : public ThreadPool::Task {
 
     // Make a copy of the state's isolate flags and hand it to the callback.
     Dart_IsolateFlags api_flags = *(state_->isolate_flags());
-    api_flags.is_system_isolate = false;
+
+    // Inherit the system isolate property to work around issues with
+    // --pause-isolates-on-start and --pause-isolates-on-exit impacting macro
+    // generation isolates which are spawned by the kernel-service
+    // (see https://github.com/dart-lang/sdk/issues/54729 for details).
+    //
+    // This flag isn't inherited in the case that the main isolate is marked as
+    // a system isolate in the standalone VM using the
+    // --mark-main-isolate-as-system-isolate flag as it's currently used to
+    // hide test runner implementation details and spawns isolates that should
+    // be debuggable as non-system isolates.
+    //
+    // TODO(bkonyi): revisit this decision, see
+    // https://github.com/dart-lang/sdk/issues/54736 for the tracking issue.
+    const bool is_parent_main_isolate =
+        strcmp(parent_isolate_->name(), "main") == 0;
+    api_flags.is_system_isolate =
+        api_flags.is_system_isolate && !is_parent_main_isolate;
     Dart_Isolate isolate =
         (create_group_callback)(state_->script_url(), name, nullptr,
                                 state_->package_config(), &api_flags,

runtime/vm/dart.cc

@@ -676,6 +676,8 @@ char* Dart::Cleanup() {
     }
   }
 
+  Isolate::KillAllSystemIsolates(Isolate::kInternalKillMsg);
+
   // Shutdown the kernel isolate.
   if (FLAG_trace_shutdown) {
     OS::PrintErr("[+%" Pd64 "ms] SHUTDOWN: Shutting down kernel isolate\n",

runtime/vm/isolate.cc

@@ -3571,11 +3571,14 @@ void Isolate::KillLocked(LibMsgId msg_id) {
 
 class IsolateKillerVisitor : public IsolateVisitor {
  public:
-  explicit IsolateKillerVisitor(Isolate::LibMsgId msg_id)
-      : target_(nullptr), msg_id_(msg_id) {}
+  IsolateKillerVisitor(Isolate::LibMsgId msg_id,
+                       bool kill_system_isolates = false)
+      : target_(nullptr),
+        msg_id_(msg_id),
+        kill_system_isolates_(kill_system_isolates) {}
 
   IsolateKillerVisitor(Isolate* isolate, Isolate::LibMsgId msg_id)
-      : target_(isolate), msg_id_(msg_id) {
+      : target_(isolate), msg_id_(msg_id), kill_system_isolates_(false) {
     ASSERT(isolate != Dart::vm_isolate());
   }
 
@@ -3593,6 +3596,11 @@ class IsolateKillerVisitor : public IsolateVisitor {
 
  private:
   bool ShouldKill(Isolate* isolate) {
+    if (kill_system_isolates_) {
+      ASSERT(target_ == nullptr);
+      // Don't kill the service isolate or vm isolate.
+      return IsSystemIsolate(isolate) && !Isolate::IsVMInternalIsolate(isolate);
+    }
     // If a target_ is specified, then only kill the target_.
     // Otherwise, don't kill the service isolate or vm isolate.
     return (((target_ != nullptr) && (isolate == target_)) ||
@@ -3601,6 +3609,7 @@ class IsolateKillerVisitor : public IsolateVisitor {
 
   Isolate* target_;
   Isolate::LibMsgId msg_id_;
+  bool kill_system_isolates_;
 };
 
 void Isolate::KillAllIsolates(LibMsgId msg_id) {
@@ -3608,6 +3617,11 @@ void Isolate::KillAllIsolates(LibMsgId msg_id) {
   VisitIsolates(&visitor);
 }
 
+void Isolate::KillAllSystemIsolates(LibMsgId msg_id) {
+  IsolateKillerVisitor visitor(msg_id, /*kill_system_isolates=*/true);
+  VisitIsolates(&visitor);
+}
+
 void Isolate::KillIfExists(Isolate* isolate, LibMsgId msg_id) {
   IsolateKillerVisitor visitor(isolate, msg_id);
   VisitIsolates(&visitor);

runtime/vm/isolate.h

@@ -1420,7 +1420,10 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
     UpdateIsolateFlagsBit<HasAttemptedSteppingBit>(value);
   }
 
+  // Kills all non-system isolates.
   static void KillAllIsolates(LibMsgId msg_id);
+  // Kills all system isolates, excluding the kernel service and VM service.
+  static void KillAllSystemIsolates(LibMsgId msg_id);
   static void KillIfExists(Isolate* isolate, LibMsgId msg_id);
 
   // Lookup an isolate by its main port. Returns nullptr if no matching isolate