Refactor Thread lifecycle interface, add Thread::Enter/ExitIsolate.

runtime/lib/isolate.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "platform/assert.h"
 #include "vm/bootstrap_natives.h"
 #include "vm/class_finalizer.h"
 #include "vm/dart.h"
 #include "vm/dart_api_impl.h"
 #include "vm/dart_entry.h"
@@ skipping 146 matching lines @@
     *error = zone->PrintToString(
         "Unable to canonicalize uri '%s': no library tag handler found.",
         uri.ToCString());
   }
   return retval;
 }
 
 
 static bool CreateIsolate(Isolate* parent_isolate,
                           IsolateSpawnState* state,
                           char** error) {

[siva, 2015/04/01 04:08:34]

Maybe assert here?

ASSERT(thread->isolate() == NULL);

[koda, 2015/04/01 16:05:59]

Already done by Thread::EnterIsolate (that's how it differs from the old
Isolate::SetCurrent).

   Dart_IsolateCreateCallback callback = Isolate::CreateCallback();
   if (callback == NULL) {
     *error = strdup("Null callback specified for isolate creation\n");
-    Isolate::SetCurrent(parent_isolate);
+    Thread::EnterIsolate(parent_isolate);
     return false;
   }
 
   void* init_data = parent_isolate->init_callback_data();
   Isolate* child_isolate = reinterpret_cast<Isolate*>(
       (callback)(state->script_url(),
                  state->function_name(),
                  state->package_root(),
                  init_data,
                  error));
   if (child_isolate == NULL) {
-    Isolate::SetCurrent(parent_isolate);
+    Thread::EnterIsolate(parent_isolate);
     return false;
   }
   if (!state->is_spawn_uri()) {
     // For isolates spawned using the spawnFunction semantics we set
     // the origin_id to the origin_id of the parent isolate.
     child_isolate->set_origin_id(parent_isolate->origin_id());
   }
   state->set_isolate(reinterpret_cast<Isolate*>(child_isolate));
 
-  Isolate::SetCurrent(parent_isolate);
+  Thread::EnterIsolate(parent_isolate);
   return true;
 }
 
 
 static void Spawn(Isolate* parent_isolate, IsolateSpawnState* state) {
+  Thread::ExitIsolate();
   // Create a new isolate.
   char* error = NULL;
   if (!CreateIsolate(parent_isolate, state, &error)) {

[siva, 2015/04/01 04:08:34]

maybe 
Thread::EnterIsolate(parent_isolate);
here and not bother with all that setting in CreateIsolate.

[koda, 2015/04/01 16:05:59]

Done.

     delete state;
     const String& msg = String::Handle(String::New(error));
     free(error);
     ThrowIsolateSpawnException(msg);
   }

[siva, 2015/04/01 04:08:34]

Thread::EnterIsolate(parent_isolate);

Seems odd that we are exiting the parent_isolate and entering it back again. Not
sure what the contract is for the isolate create callback, need to check if we
guarantee that the current isolate 

[koda, 2015/04/01 16:05:59]

This stems from the spec of the API method Dart_CreateIsolate: it enters the
newly created isolate, and requires there to be no current isolate when called.
And the spec for the callback says it's expected to call Dart_CreateIsolate, so
it's natural that the same pre/post-conditions apply (although we could be more
explicit about it).

I agree that the current interface is awkward in this case. We could add a flag
to Dart_CreateIsolate that inhibits entering the isolate.

But, since this is meant to be a mostly refactoring CL, I'd rather save changes
to the external API spec for later.

 
   // Start the new isolate if it is already marked as runnable.
   Isolate* spawned_isolate = state->isolate();
   MutexLocker ml(spawned_isolate->mutex());
   spawned_isolate->set_spawn_state(state);
   if (spawned_isolate->is_runnable()) {
     spawned_isolate->Run();
   }
 }
 
@@ skipping 85 matching lines @@
   MessageWriter writer(&data, &allocator, false);
   writer.WriteMessage(msg);
 
   PortMap::PostMessage(new Message(port.Id(),
                                    data, writer.BytesWritten(),
                                    Message::kOOBPriority));
   return Object::null();
 }
 
 }  // namespace dart