[VM] - Fix race condition in setting kernel isolate

(was being added to the isolate list before it was marked as a kernel isolate)

Also do not send service events or debug events from the kernel isolate

Change-Id: I82a1dac3c8a845344dd26ce8a2a9077eb93f8447
Reviewed-on: https://dart-review.googlesource.com/65881
Commit-Queue: Siva Annamalai <asiva@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>

runtime/bin/main.cc

@@ -503,9 +503,9 @@ static Dart_Isolate CreateAndSetupKernelIsolate(const char* script_uri,
     isolate_data->set_kernel_buffer(const_cast<uint8_t*>(kernel_service_buffer),
                                     kernel_service_buffer_size,
                                     false /* take_ownership */);
-    isolate = Dart_CreateIsolateFromKernel(uri, main, kernel_service_buffer,
-                                           kernel_service_buffer_size, flags,
-                                           isolate_data, error);
+    isolate = Dart_CreateIsolateFromKernel(
+        DART_KERNEL_ISOLATE_NAME, main, kernel_service_buffer,
+        kernel_service_buffer_size, flags, isolate_data, error);
   }
 
   if (isolate == NULL) {

runtime/vm/dart.cc

@@ -619,18 +619,11 @@ RawError* Dart::InitializeIsolate(const uint8_t* snapshot_data,
   if (FLAG_print_class_table) {
     I->class_table()->Print();
   }
-
-  bool is_kernel_isolate = false;
-  USE(is_kernel_isolate);
-
-#ifndef DART_PRECOMPILED_RUNTIME
-  KernelIsolate::InitCallback(I);
-  is_kernel_isolate = KernelIsolate::IsKernelIsolate(I);
-#endif
-
   ServiceIsolate::MaybeMakeServiceIsolate(I);
+
 #if !defined(PRODUCT)
-  if (!ServiceIsolate::IsServiceIsolate(I) && !is_kernel_isolate) {
+  if (!ServiceIsolate::IsServiceIsolate(I) &&
+      !KernelIsolate::IsKernelIsolate(I)) {
     I->message_handler()->set_should_pause_on_start(
         FLAG_pause_isolates_on_start);
     I->message_handler()->set_should_pause_on_exit(FLAG_pause_isolates_on_exit);

runtime/vm/debugger.cc

@@ -324,14 +324,12 @@ ActivationFrame::ActivationFrame(const Closure& async_activation)
 }
 
 bool Debugger::NeedsIsolateEvents() {
-  return ((isolate_ != Dart::vm_isolate()) &&
-          !ServiceIsolate::IsServiceIsolateDescendant(isolate_) &&
+  return (!Isolate::IsVMInternalIsolate(isolate_) &&
           ((event_handler_ != NULL) || Service::isolate_stream.enabled()));
 }
 
 bool Debugger::NeedsDebugEvents() {
-  ASSERT(isolate_ != Dart::vm_isolate() &&
-         !ServiceIsolate::IsServiceIsolateDescendant(isolate_));
+  ASSERT(!Isolate::IsVMInternalIsolate(isolate_));
   return (FLAG_warn_on_pause_with_no_debugger || (event_handler_ != NULL) ||
           Service::debug_stream.enabled());
 }
@@ -1735,8 +1733,7 @@ Debugger::~Debugger() {
 void Debugger::Shutdown() {
   // TODO(johnmccutchan): Do not create a debugger for isolates that don't need
   // them. Then, assert here that isolate_ is not one of those isolates.
-  if ((isolate_ == Dart::vm_isolate()) ||
-      ServiceIsolate::IsServiceIsolateDescendant(isolate_)) {
+  if (Isolate::IsVMInternalIsolate(isolate_)) {
     return;
   }
   while (breakpoint_locations_ != NULL) {

runtime/vm/isolate.cc

@@ -1055,10 +1055,17 @@ Isolate* Isolate::Init(const char* name_prefix,
 #undef ISOLATE_METRIC_INIT
 #endif  // !defined(PRODUCT)
 
-  bool is_service_or_kernel_isolate = ServiceIsolate::NameEquals(name_prefix);
+  bool is_service_or_kernel_isolate = false;
+  if (ServiceIsolate::NameEquals(name_prefix)) {
+    ASSERT(!ServiceIsolate::Exists());
+    is_service_or_kernel_isolate = true;
+  }
 #if !defined(DART_PRECOMPILED_RUNTIME)
-  is_service_or_kernel_isolate =
-      is_service_or_kernel_isolate || KernelIsolate::NameEquals(name_prefix);
+  if (KernelIsolate::NameEquals(name_prefix)) {
+    ASSERT(!KernelIsolate::Exists());
+    KernelIsolate::SetKernelIsolate(result);
+    is_service_or_kernel_isolate = true;
+  }
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
   Heap::Init(result,
@@ -1122,6 +1129,12 @@ Isolate* Isolate::Init(const char* name_prefix,
   if (!AddIsolateToList(result)) {
     result->LowLevelShutdown();
     Thread::ExitIsolate();
+    if (KernelIsolate::IsKernelIsolate(result)) {
+      KernelIsolate::SetKernelIsolate(NULL);
+    }
+    if (ServiceIsolate::IsServiceIsolate(result)) {
+      ServiceIsolate::SetServiceIsolate(NULL);
+    }
     delete result;
     return NULL;
   }
@@ -1221,7 +1234,7 @@ void Isolate::DoneLoading() {
 
 #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
 bool Isolate::CanReload() const {
-  return !ServiceIsolate::IsServiceIsolateDescendant(this) && is_runnable() &&
+  return !Isolate::IsVMInternalIsolate(this) && is_runnable() &&
          !IsReloading() &&
          (AtomicOperations::LoadRelaxed(&no_reload_scope_depth_) == 0) &&
          IsolateCreationEnabled() &&
@@ -1275,7 +1288,7 @@ const char* Isolate::MakeRunnable() {
   ASSERT(object_store()->root_library() != Library::null());
   set_is_runnable(true);
 #ifndef PRODUCT
-  if (!ServiceIsolate::IsServiceIsolate(this)) {
+  if (!Isolate::IsVMInternalIsolate(this)) {
     debugger()->OnIsolateRunnable();
     if (FLAG_pause_isolates_on_unhandled_exceptions) {
       debugger()->SetExceptionPauseInfo(kPauseOnUnhandledExceptions);
@@ -1833,8 +1846,7 @@ void Isolate::Shutdown() {
 
     // Write compiler stats data if enabled.
     if (FLAG_support_compiler_stats && FLAG_compiler_stats &&
-        !ServiceIsolate::IsServiceIsolateDescendant(this) &&
-        (this != Dart::vm_isolate())) {
+        !Isolate::IsVMInternalIsolate(this)) {
       OS::PrintErr("%s", aggregate_compiler_stats()->PrintToZone());
     }
   }
@@ -1862,9 +1874,8 @@ void Isolate::Shutdown() {
   }
 
 #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
-  if (FLAG_check_reloaded && is_runnable() && (this != Dart::vm_isolate()) &&
-      !KernelIsolate::IsKernelIsolate(this) &&
-      !ServiceIsolate::IsServiceIsolateDescendant(this)) {
+  if (FLAG_check_reloaded && is_runnable() &&
+      !Isolate::IsVMInternalIsolate(this)) {
     if (!HasAttemptedReload()) {
       FATAL(
           "Isolate did not reload before exiting and "
@@ -2430,6 +2441,9 @@ void Isolate::RegisterServiceExtensionHandler(const String& name,
   if (!FLAG_support_service) {
     return;
   }
+  if (Isolate::IsVMInternalIsolate(this)) {
+    return;
+  }
   GrowableObjectArray& handlers =
       GrowableObjectArray::Handle(registered_service_extension_handlers());
   if (handlers.IsNull()) {
@@ -2620,7 +2634,7 @@ bool Isolate::IsolateCreationEnabled() {
   return creation_enabled_;
 }
 
-bool Isolate::IsVMInternalIsolate(Isolate* isolate) {
+bool Isolate::IsVMInternalIsolate(const Isolate* isolate) {
   return (isolate == Dart::vm_isolate()) ||
          ServiceIsolate::IsServiceIsolateDescendant(isolate) ||
          KernelIsolate::IsKernelIsolate(isolate);

runtime/vm/isolate.h

@@ -762,7 +762,7 @@ class Isolate : public BaseIsolate {
   static void DisableIsolateCreation();
   static void EnableIsolateCreation();
   static bool IsolateCreationEnabled();
-  static bool IsVMInternalIsolate(Isolate* isolate);
+  static bool IsVMInternalIsolate(const Isolate* isolate);
 
 #if !defined(PRODUCT)
   intptr_t reload_every_n_stack_overflow_checks() const {

runtime/vm/kernel_isolate.h

@@ -83,6 +83,7 @@ class KernelIsolate : public AllStatic {
   }
 
   friend class Dart;
+  friend class Isolate;
   friend class RunKernelTask;
 };
 

runtime/vm/runtime_entry.cc

@@ -1877,15 +1877,7 @@ static void HandleStackOverflowTestCases(Thread* thread) {
       isolate->reload_every_n_stack_overflow_checks();
   if ((FLAG_deoptimize_every > 0) || (FLAG_stacktrace_every > 0) ||
       (isolate_reload_every > 0)) {
-    bool is_auxiliary_isolate = ServiceIsolate::IsServiceIsolate(isolate);
-#if !defined(DART_PRECOMPILED_RUNTIME)
-    // Certain flags should not effect the kernel isolate itself.  They might be
-    // used by tests via the "VMOptions=--..." annotation to test VM
-    // functionality in the main isolate.
-    is_auxiliary_isolate =
-        is_auxiliary_isolate || KernelIsolate::IsKernelIsolate(isolate);
-#endif  // !defined(DART_PRECOMPILED_RUNTIME)
-    if (!is_auxiliary_isolate) {
+    if (!Isolate::IsVMInternalIsolate(isolate)) {
       // TODO(turnidge): To make --deoptimize_every and
       // --stacktrace-every faster we could move this increment/test to
       // the generated code.

runtime/vm/service.cc

@@ -931,7 +931,7 @@ void Service::SendEvent(const char* stream_id,
   Thread* thread = Thread::Current();
   Isolate* isolate = thread->isolate();
   ASSERT(isolate != NULL);
-  ASSERT(!ServiceIsolate::IsServiceIsolateDescendant(isolate));
+  ASSERT(!Isolate::IsVMInternalIsolate(isolate));
 
   if (FLAG_trace_service) {
     OS::PrintErr(

runtime/vm/service_event.cc

@@ -33,8 +33,7 @@ ServiceEvent::ServiceEvent(Isolate* isolate, EventKind event_kind)
       timestamp_(OS::GetCurrentTimeMillis()) {
   // We should never generate events for the vm or service isolates.
   ASSERT(isolate_ != Dart::vm_isolate());
-  ASSERT(isolate == NULL ||
-         !ServiceIsolate::IsServiceIsolateDescendant(isolate_));
+  ASSERT(isolate == NULL || !Isolate::IsVMInternalIsolate(isolate));
 
   if ((event_kind == ServiceEvent::kPauseStart) ||
       (event_kind == ServiceEvent::kPauseExit)) {

runtime/vm/service_isolate.cc

@@ -194,7 +194,7 @@ bool ServiceIsolate::SendIsolateStartupMessage() {
   }
   Thread* thread = Thread::Current();
   Isolate* isolate = thread->isolate();
-  if (IsServiceIsolateDescendant(isolate)) {
+  if (Isolate::IsVMInternalIsolate(isolate)) {
     return false;
   }
   ASSERT(isolate != NULL);
@@ -219,7 +219,7 @@ bool ServiceIsolate::SendIsolateShutdownMessage() {
   }
   Thread* thread = Thread::Current();
   Isolate* isolate = thread->isolate();
-  if (IsServiceIsolateDescendant(isolate)) {
+  if (Isolate::IsVMInternalIsolate(isolate)) {
     return false;
   }
   ASSERT(isolate != NULL);

runtime/vm/service_isolate.h

@@ -78,6 +78,7 @@ class ServiceIsolate : public AllStatic {
   static char* server_address_;
 
   friend class Dart;
+  friend class Isolate;
   friend class RunServiceTask;
   friend class ServiceIsolateNatives;
 };