[vm] Add Native->VM transitions when dereferencing API handles.

For example setting return value unwraps handles and stores
raw pointer values to the stack which might race with GC
in another thread.

This CL adds assertions in helper methods from Api class which
unwrap API handles and fixes all places that were revealed by
those assertions.

Caveat: we still permit to check whether handle contains
Smi or not without entering VM state - because GC does not
change this property.

Bug: b/127482366
Change-Id: I59f08c2a91935995514fb70607c2777aa2844d94
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/95654
Commit-Queue: Siva Annamalai <asiva@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc
index f0e7376..3e81020 100644
--- a/runtime/vm/dart_api_impl.cc
+++ b/runtime/vm/dart_api_impl.cc
@@ -359,6 +359,7 @@
 RawObject* Api::UnwrapHandle(Dart_Handle object) {
 #if defined(DEBUG)
   Thread* thread = Thread::Current();
+  ASSERT(thread->execution_state() == Thread::kThreadInVM);
   ASSERT(thread->IsMutatorThread());
   ASSERT(thread->isolate() != NULL);
   ASSERT(!FLAG_verify_handles || thread->IsValidLocalHandle(object) ||
@@ -694,14 +695,20 @@
 // --- Handles ---
 
 DART_EXPORT bool Dart_IsError(Dart_Handle handle) {
+  Thread* thread = Thread::Current();
+  TransitionNativeToVM transition(thread);
   return Api::IsError(handle);
 }
 
 DART_EXPORT bool Dart_IsApiError(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(object) == kApiErrorCid;
 }
 
 DART_EXPORT bool Dart_IsUnhandledExceptionError(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(object) == kUnhandledExceptionCid;
 }
 
@@ -713,10 +720,15 @@
     const Instance& exc = Instance::Handle(Z, error.exception());
     return IsCompiletimeErrorObject(Z, exc);
   }
+
+  Thread* thread = Thread::Current();
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(object) == kLanguageErrorCid;
 }
 
 DART_EXPORT bool Dart_IsFatalError(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(object) == kUnwindErrorCid;
 }
 
@@ -1457,14 +1469,17 @@
   Isolate* isolate = thread->isolate();
   CHECK_ISOLATE(isolate);
   NoSafepointScope no_safepoint_scope;
-  if ((isolate->sticky_error() != Error::null()) && !::Dart_IsNull(error)) {
+  const Error& error_handle = Api::UnwrapErrorHandle(Z, error);
+  if ((isolate->sticky_error() != Error::null()) &&
+      (error_handle.raw() != Object::null())) {
     FATAL1("%s expects there to be no sticky error.", CURRENT_FUNC);
   }
-  if (!::Dart_IsUnhandledExceptionError(error) && !::Dart_IsNull(error)) {
+  if (!error_handle.IsUnhandledException() &&
+      (error_handle.raw() != Object::null())) {
     FATAL1("%s expects the error to be an unhandled exception error or null.",
            CURRENT_FUNC);
   }
-  isolate->SetStickyError(Api::UnwrapErrorHandle(Z, error).raw());
+  isolate->SetStickyError(error_handle.raw());
 }
 
 DART_EXPORT bool Dart_HasStickyError() {
@@ -1541,7 +1556,7 @@
   CHECK_NULL(isolate_snapshot_data_size);
   // Finalize all classes if needed.
   Dart_Handle state = Api::CheckAndFinalizePendingClasses(T);
-  if (::Dart_IsError(state)) {
+  if (Api::IsError(state)) {
     return state;
   }
   BackgroundCompiler::Stop(I);
@@ -1877,6 +1892,7 @@
 }
 
 DART_EXPORT bool Dart_IsNull(Dart_Handle object) {
+  TransitionNativeToVM transition(Thread::Current());
   return Api::UnwrapHandle(object) == Object::null();
 }
 
@@ -1936,6 +1952,7 @@
 DART_EXPORT bool Dart_IsInstance(Dart_Handle object) {
   Thread* thread = Thread::Current();
   CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   REUSABLE_OBJECT_HANDLESCOPE(thread);
   Object& ref = thread->ObjectHandle();
   ref = Api::UnwrapHandle(object);
@@ -1943,39 +1960,60 @@
 }
 
 DART_EXPORT bool Dart_IsNumber(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return RawObject::IsNumberClassId(Api::ClassId(object));
 }
 
 DART_EXPORT bool Dart_IsInteger(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return RawObject::IsIntegerClassId(Api::ClassId(object));
 }
 
 DART_EXPORT bool Dart_IsDouble(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(object) == kDoubleCid;
 }
 
 DART_EXPORT bool Dart_IsBoolean(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(object) == kBoolCid;
 }
 
 DART_EXPORT bool Dart_IsString(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return RawObject::IsStringClassId(Api::ClassId(object));
 }
 
 DART_EXPORT bool Dart_IsStringLatin1(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return RawObject::IsOneByteStringClassId(Api::ClassId(object));
 }
 
 DART_EXPORT bool Dart_IsExternalString(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return RawObject::IsExternalStringClassId(Api::ClassId(object));
 }
 
 DART_EXPORT bool Dart_IsList(Dart_Handle object) {
+  DARTSCOPE(Thread::Current());
   if (RawObject::IsBuiltinListClassId(Api::ClassId(object))) {
     return true;
   }
 
-  DARTSCOPE(Thread::Current());
   const Object& obj = Object::Handle(Z, Api::UnwrapHandle(object));
   return GetListInstance(Z, obj) != Instance::null();
 }
@@ -1987,26 +2025,44 @@
 }
 
 DART_EXPORT bool Dart_IsLibrary(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(object) == kLibraryCid;
 }
 
 DART_EXPORT bool Dart_IsType(Dart_Handle handle) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(handle) == kTypeCid;
 }
 
 DART_EXPORT bool Dart_IsFunction(Dart_Handle handle) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(handle) == kFunctionCid;
 }
 
 DART_EXPORT bool Dart_IsVariable(Dart_Handle handle) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(handle) == kFieldCid;
 }
 
 DART_EXPORT bool Dart_IsTypeVariable(Dart_Handle handle) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(handle) == kTypeParameterCid;
 }
 
 DART_EXPORT bool Dart_IsClosure(Dart_Handle object) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(object) == kClosureCid;
 }
 
@@ -2023,6 +2079,9 @@
 }
 
 DART_EXPORT bool Dart_IsTypedData(Dart_Handle handle) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   intptr_t cid = Api::ClassId(handle);
   return RawObject::IsTypedDataClassId(cid) ||
          RawObject::IsExternalTypedDataClassId(cid) ||
@@ -2030,6 +2089,9 @@
 }
 
 DART_EXPORT bool Dart_IsByteBuffer(Dart_Handle handle) {
+  Thread* thread = Thread::Current();
+  CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   return Api::ClassId(handle) == kByteBufferCid;
 }
 
@@ -2170,13 +2232,16 @@
   API_TIMELINE_DURATION(thread);
   Isolate* isolate = thread->isolate();
   CHECK_ISOLATE(isolate);
-  intptr_t class_id = Api::ClassId(integer);
-  if (class_id == kSmiCid || class_id == kMintCid) {
+  if (Api::IsSmi(integer)) {
     *fits = true;
     return Api::Success();
   }
-  // Slow path for type error.
+  // Slow path for mints and type error.
   DARTSCOPE(thread);
+  if (Api::ClassId(integer) == kMintCid) {
+    *fits = true;
+    return Api::Success();
+  }
   const Integer& int_obj = Api::UnwrapIntegerHandle(Z, integer);
   ASSERT(int_obj.IsNull());
   RETURN_TYPE_ERROR(Z, integer, Integer);
@@ -2602,6 +2667,7 @@
                                                intptr_t* size) {
   Thread* thread = Thread::Current();
   CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   ReusableObjectHandleScope reused_obj_handle(thread);
   const String& str_obj = Api::UnwrapStringHandle(reused_obj_handle, str);
   if (str_obj.IsNull()) {
@@ -2620,6 +2686,7 @@
                                                  void** peer) {
   Thread* thread = Thread::Current();
   CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   ReusableObjectHandleScope reused_obj_handle(thread);
   const String& str = Api::UnwrapStringHandle(reused_obj_handle, object);
   if (str.IsNull()) {
@@ -2830,7 +2897,7 @@
           args.SetAt(1, index);
           Dart_Handle value =
               Api::NewHandle(T, DartEntry::InvokeFunction(function, args));
-          if (::Dart_IsError(value)) return value;
+          if (Api::IsError(value)) return value;
           result[i] = value;
         }
         return Api::Success();
@@ -3299,7 +3366,9 @@
 }
 
 DART_EXPORT Dart_TypedData_Type Dart_GetTypeOfTypedData(Dart_Handle object) {
-  API_TIMELINE_DURATION(Thread::Current());
+  Thread* thread = Thread::Current();
+  API_TIMELINE_DURATION(thread);
+  TransitionNativeToVM transition(thread);
   intptr_t class_id = Api::ClassId(object);
   if (RawObject::IsTypedDataClassId(class_id) ||
       RawObject::IsTypedDataViewClassId(class_id)) {
@@ -3310,14 +3379,16 @@
 
 DART_EXPORT Dart_TypedData_Type
 Dart_GetTypeOfExternalTypedData(Dart_Handle object) {
-  API_TIMELINE_DURATION(Thread::Current());
+  Thread* thread = Thread::Current();
+  API_TIMELINE_DURATION(thread);
+  TransitionNativeToVM transition(thread);
   intptr_t class_id = Api::ClassId(object);
   if (RawObject::IsExternalTypedDataClassId(class_id)) {
     return GetType(class_id);
   }
   if (RawObject::IsTypedDataViewClassId(class_id)) {
     // Check if data object of the view is external.
-    Zone* zone = Thread::Current()->zone();
+    Zone* zone = thread->zone();
     const Instance& view_obj = Api::UnwrapInstanceHandle(zone, object);
     ASSERT(!view_obj.IsNull());
     const Instance& data_obj =
@@ -3401,7 +3472,7 @@
   Dart_Handle ext_data =
       NewExternalTypedData(thread, kExternalTypedDataUint8ArrayCid, data,
                            length, peer, external_allocation_size, callback);
-  if (::Dart_IsError(ext_data)) {
+  if (Api::IsError(ext_data)) {
     return ext_data;
   }
   Object& result = Object::Handle(zone);
@@ -4109,7 +4180,7 @@
     Array& args = Array::Handle(Z);
     result =
         SetupArguments(T, number_of_arguments, arguments, extra_args, &args);
-    if (!::Dart_IsError(result)) {
+    if (!Api::IsError(result)) {
       args.SetAt(0, instance);
       const Object& retval =
           Object::Handle(Z, DartEntry::InvokeFunction(constructor, args));
@@ -4168,7 +4239,7 @@
 
     // Setup args and check for malformed arguments in the arguments list.
     result = SetupArguments(T, number_of_arguments, arguments, 0, &args);
-    if (::Dart_IsError(result)) {
+    if (Api::IsError(result)) {
       return result;
     }
     return Api::NewHandle(
@@ -4183,7 +4254,7 @@
 
     // Setup args and check for malformed arguments in the arguments list.
     result = SetupArguments(T, number_of_arguments, arguments, 1, &args);
-    if (::Dart_IsError(result)) {
+    if (Api::IsError(result)) {
       return result;
     }
     args.SetAt(0, instance);
@@ -4206,7 +4277,7 @@
 
     // Setup args and check for malformed arguments in the arguments list.
     result = SetupArguments(T, number_of_arguments, arguments, 0, &args);
-    if (::Dart_IsError(result)) {
+    if (Api::IsError(result)) {
       return result;
     }
 
@@ -4401,7 +4472,7 @@
   Isolate* isolate = thread->isolate();
   CHECK_ISOLATE(isolate);
   CHECK_CALLBACK_STATE(thread);
-  if (Api::IsError(exception)) {
+  if (::Dart_IsError(exception)) {
     ::Dart_PropagateError(exception);
   }
   TransitionNativeToVM transition(thread);
@@ -4475,6 +4546,7 @@
                                                          int* count) {
   Thread* thread = Thread::Current();
   CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   ReusableObjectHandleScope reused_obj_handle(thread);
   const Instance& instance = Api::UnwrapInstanceHandle(reused_obj_handle, obj);
   if (instance.IsNull()) {
@@ -4489,6 +4561,7 @@
                                                     intptr_t* value) {
   Thread* thread = Thread::Current();
   CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   ReusableObjectHandleScope reused_obj_handle(thread);
   const Instance& instance = Api::UnwrapInstanceHandle(reused_obj_handle, obj);
   if (instance.IsNull()) {
@@ -4783,11 +4856,11 @@
   NativeArguments* arguments = reinterpret_cast<NativeArguments*>(args);
   ASSERT(arguments->thread()->isolate() == Isolate::Current());
   ASSERT_CALLBACK_STATE(arguments->thread());
+  TransitionNativeToVM transition(arguments->thread());
   if ((retval != Api::Null()) && !Api::IsInstance(retval) &&
       !Api::IsError(retval)) {
     // Print the current stack trace to make the problematic caller
     // easier to find.
-    TransitionNativeToVM transition(arguments->thread());
     const StackTrace& stacktrace = GetCurrentStackTrace(0);
     OS::PrintErr("=== Current Trace:\n%s===\n", stacktrace.ToCString());
 
@@ -4804,6 +4877,7 @@
 DART_EXPORT void Dart_SetWeakHandleReturnValue(Dart_NativeArguments args,
                                                Dart_WeakPersistentHandle rval) {
   NativeArguments* arguments = reinterpret_cast<NativeArguments*>(args);
+  TransitionNativeToVM transition(arguments->thread());
 #if defined(DEBUG)
   Isolate* isolate = arguments->thread()->isolate();
   ASSERT(isolate == Isolate::Current());
@@ -4915,6 +4989,7 @@
 DART_EXPORT void Dart_SetBooleanReturnValue(Dart_NativeArguments args,
                                             bool retval) {
   NativeArguments* arguments = reinterpret_cast<NativeArguments*>(args);
+  TransitionNativeToVM transition(arguments->thread());
   ASSERT(arguments->thread()->isolate() == Isolate::Current());
   ASSERT_CALLBACK_STATE(arguments->thread());
   arguments->SetReturn(Bool::Get(retval));
@@ -4923,14 +4998,13 @@
 DART_EXPORT void Dart_SetIntegerReturnValue(Dart_NativeArguments args,
                                             int64_t retval) {
   NativeArguments* arguments = reinterpret_cast<NativeArguments*>(args);
+  TransitionNativeToVM transition(arguments->thread());
   ASSERT(arguments->thread()->isolate() == Isolate::Current());
   ASSERT_CALLBACK_STATE(arguments->thread());
   if (Smi::IsValid(retval)) {
     Api::SetSmiReturnValue(arguments, static_cast<intptr_t>(retval));
   } else {
     // Slow path for Mints.
-    ASSERT_CALLBACK_STATE(arguments->thread());
-    TransitionNativeToVM transition(arguments->thread());
     Api::SetIntegerReturnValue(arguments, retval);
   }
 }
@@ -5316,7 +5390,7 @@
 
   // Finalize all classes if needed.
   Dart_Handle state = Api::CheckAndFinalizePendingClasses(T);
-  if (::Dart_IsError(state)) {
+  if (Api::IsError(state)) {
     return state;
   }
 
@@ -5416,6 +5490,7 @@
   }
   Thread* thread = Thread::Current();
   CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   REUSABLE_OBJECT_HANDLESCOPE(thread);
   Object& obj = thread->ObjectHandle();
   obj = Api::UnwrapHandle(object);
@@ -5435,6 +5510,7 @@
 DART_EXPORT Dart_Handle Dart_SetPeer(Dart_Handle object, void* peer) {
   Thread* thread = Thread::Current();
   CHECK_ISOLATE(thread->isolate());
+  TransitionNativeToVM transition(thread);
   REUSABLE_OBJECT_HANDLESCOPE(thread);
   Object& obj = thread->ObjectHandle();
   obj = Api::UnwrapHandle(object);
@@ -5929,7 +6005,7 @@
     return Api::NewError("Flag --precompilation was not specified.");
   }
   Dart_Handle result = Api::CheckAndFinalizePendingClasses(T);
-  if (::Dart_IsError(result)) {
+  if (Api::IsError(result)) {
     return result;
   }
   CHECK_CALLBACK_STATE(T);
@@ -6151,7 +6227,7 @@
   CHECK_NULL(isolate_snapshot_instructions_size);
   // Finalize all classes if needed.
   Dart_Handle state = Api::CheckAndFinalizePendingClasses(T);
-  if (::Dart_IsError(state)) {
+  if (Api::IsError(state)) {
     return state;
   }
   BackgroundCompiler::Stop(I);
@@ -6210,7 +6286,7 @@
   CHECK_NULL(isolate_snapshot_instructions_size);
   // Finalize all classes if needed.
   Dart_Handle state = Api::CheckAndFinalizePendingClasses(T);
-  if (::Dart_IsError(state)) {
+  if (Api::IsError(state)) {
     return state;
   }
   BackgroundCompiler::Stop(I);
diff --git a/runtime/vm/dart_api_impl.h b/runtime/vm/dart_api_impl.h
index 570a855..76a6b0d 100644
--- a/runtime/vm/dart_api_impl.h
+++ b/runtime/vm/dart_api_impl.h
@@ -186,11 +186,20 @@
 
   // Returns true if the handle holds a Smi.
   static bool IsSmi(Dart_Handle handle) {
-    // TODO(turnidge): Assumes RawObject* is at offset zero.  Fix.
+    // Important: we do not require current thread to be in VM state because
+    // we do not dereference the handle.
     RawObject* raw = *(reinterpret_cast<RawObject**>(handle));
     return !raw->IsHeapObject();
   }
 
+  // Returns the value of a Smi.
+  static intptr_t SmiValue(Dart_Handle handle) {
+    // Important: we do not require current thread to be in VM state because
+    // we do not dereference the handle.
+    RawObject* value = *(reinterpret_cast<RawObject**>(handle));
+    return Smi::Value(static_cast<RawSmi*>(value));
+  }
+
   // Returns true if the handle holds a Dart Instance.
   static bool IsInstance(Dart_Handle handle) {
     return (ClassId(handle) >= kInstanceCid);
@@ -204,16 +213,8 @@
     return RawObject::IsErrorClassId(ClassId(handle));
   }
 
-  // Returns the value of a Smi.
-  static intptr_t SmiValue(Dart_Handle handle) {
-    // TODO(turnidge): Assumes RawObject* is at offset zero.  Fix.
-    uword value = *(reinterpret_cast<uword*>(handle));
-    return Smi::ValueFromRaw(value);
-  }
-
   static intptr_t ClassId(Dart_Handle handle) {
-    // TODO(turnidge): Assumes RawObject* is at offset zero.  Fix.
-    RawObject* raw = *(reinterpret_cast<RawObject**>(handle));
+    RawObject* raw = UnwrapHandle(handle);
     if (!raw->IsHeapObject()) {
       return kSmiCid;
     }
diff --git a/runtime/vm/guard_field_test.cc b/runtime/vm/guard_field_test.cc
index bc30357..be010ce 100644
--- a/runtime/vm/guard_field_test.cc
+++ b/runtime/vm/guard_field_test.cc
@@ -224,8 +224,8 @@
       "}\n";
   Dart_Handle lib = TestCase::LoadTestScript(script_chars, NULL);
   Dart_Handle result = Dart_Invoke(lib, NewString("main"), 0, NULL);
-  TransitionNativeToVM transition(thread);
   EXPECT_VALID(result);
+  TransitionNativeToVM transition(thread);
   Field& f1 = Field::ZoneHandle(LookupField(lib, "A", "f1"));
   Field& f2 = Field::ZoneHandle(LookupField(lib, "A", "f2"));
   Field& f3 = Field::ZoneHandle(LookupField(lib, "A", "f3"));
diff --git a/runtime/vm/handles_test.cc b/runtime/vm/handles_test.cc
index 8dcb1ba..142e147 100644
--- a/runtime/vm/handles_test.cc
+++ b/runtime/vm/handles_test.cc
@@ -92,7 +92,10 @@
     TransitionNativeToVM transition(thread);
     StackZone sz(thread);
     handle = reinterpret_cast<Dart_Handle>(&Smi::ZoneHandle(Smi::New(1)));
-    EXPECT_VALID(handle);
+    {
+      TransitionVMToNative to_native(thread);
+      EXPECT_VALID(handle);
+    }
   }
   EXPECT(!Api::IsValid(handle));
 
@@ -103,7 +106,10 @@
       TransitionNativeToVM transition(thread);
       HANDLESCOPE(thread);
       handle = reinterpret_cast<Dart_Handle>(&Smi::Handle(Smi::New(1)));
-      EXPECT_VALID(handle);
+      {
+        TransitionVMToNative to_native(thread);
+        EXPECT_VALID(handle);
+      }
     }
     Dart_ExitScope();
   }
diff --git a/runtime/vm/json_test.cc b/runtime/vm/json_test.cc
index c42d2e6..1746328 100644
--- a/runtime/vm/json_test.cc
+++ b/runtime/vm/json_test.cc
@@ -218,26 +218,35 @@
   Dart_Handle result;
   TransitionNativeToVM transition1(thread);
   String& obj = String::Handle();
-  TransitionVMToNative transition2(thread);
 
-  {
-    result = Dart_GetField(lib, NewString("ascii"));
-    EXPECT_VALID(result);
+  auto do_test = [&](const char* field_name, const char* expected) {
+    {
+      TransitionVMToNative to_native(thread);
+      result = Dart_GetField(lib, NewString(field_name));
+      EXPECT_VALID(result);
+    }
+
     obj ^= Api::UnwrapHandle(result);
 
-    JSONStream js;
     {
-      JSONObject jsobj(&js);
-      EXPECT(!jsobj.AddPropertyStr("ascii", obj));
+      JSONStream js;
+      {
+        JSONObject jsobj(&js);
+        EXPECT(!jsobj.AddPropertyStr(field_name, obj));
+      }
+      EXPECT_STREQ(expected, js.ToCString());
     }
-    EXPECT_STREQ("{\"ascii\":\"Hello, World!\"}", js.ToCString());
+  };
+
+  {
+    TransitionVMToNative to_native(thread);
+    result = Dart_GetField(lib, NewString("ascii"));
+    EXPECT_VALID(result);
   }
 
-  {
-    result = Dart_GetField(lib, NewString("ascii"));
-    EXPECT_VALID(result);
-    obj ^= Api::UnwrapHandle(result);
+  obj ^= Api::UnwrapHandle(result);
 
+  {
     JSONStream js;
     {
       JSONObject jsobj(&js);
@@ -246,58 +255,11 @@
     EXPECT_STREQ("{\"subrange\":\"ello\"}", js.ToCString());
   }
 
-  {
-    result = Dart_GetField(lib, NewString("unicode"));
-    EXPECT_VALID(result);
-    obj ^= Api::UnwrapHandle(result);
-
-    JSONStream js;
-    {
-      JSONObject jsobj(&js);
-      EXPECT(!jsobj.AddPropertyStr("unicode", obj));
-    }
-    EXPECT_STREQ("{\"unicode\":\"Îñţérñåţîöñåļîžåţîờñ\"}", js.ToCString());
-  }
-
-  {
-    result = Dart_GetField(lib, NewString("surrogates"));
-    EXPECT_VALID(result);
-    obj ^= Api::UnwrapHandle(result);
-
-    JSONStream js;
-    {
-      JSONObject jsobj(&js);
-      EXPECT(!jsobj.AddPropertyStr("surrogates", obj));
-    }
-    EXPECT_STREQ("{\"surrogates\":\"𝄞𝄞𝄞𝄞𝄞\"}", js.ToCString());
-  }
-
-  {
-    result = Dart_GetField(lib, NewString("wrongEncoding"));
-    EXPECT_VALID(result);
-    obj ^= Api::UnwrapHandle(result);
-
-    JSONStream js;
-    {
-      JSONObject jsobj(&js);
-      EXPECT(!jsobj.AddPropertyStr("wrongEncoding", obj));
-    }
-    EXPECT_STREQ("{\"wrongEncoding\":\"𝄞\\uD834𝄞\"}", js.ToCString());
-  }
-
-  {
-    result = Dart_GetField(lib, NewString("nullInMiddle"));
-    EXPECT_VALID(result);
-    obj ^= Api::UnwrapHandle(result);
-
-    JSONStream js;
-    {
-      JSONObject jsobj(&js);
-      EXPECT(!jsobj.AddPropertyStr("nullInMiddle", obj));
-    }
-    EXPECT_STREQ("{\"nullInMiddle\":\"This has\\u0000 four words.\"}",
-                 js.ToCString());
-  }
+  do_test("ascii", "{\"ascii\":\"Hello, World!\"}");
+  do_test("unicode", "{\"unicode\":\"Îñţérñåţîöñåļîžåţîờñ\"}");
+  do_test("surrogates", "{\"surrogates\":\"𝄞𝄞𝄞𝄞𝄞\"}");
+  do_test("wrongEncoding", "{\"wrongEncoding\":\"𝄞\\uD834𝄞\"}");
+  do_test("nullInMiddle", "{\"nullInMiddle\":\"This has\\u0000 four words.\"}");
 }
 
 TEST_CASE(JSON_JSONStream_Params) {
diff --git a/runtime/vm/native_api_impl.cc b/runtime/vm/native_api_impl.cc
index 6f12a68..92192fd 100644
--- a/runtime/vm/native_api_impl.cc
+++ b/runtime/vm/native_api_impl.cc
@@ -185,7 +185,7 @@
   DARTSCOPE(Thread::Current());
   API_TIMELINE_DURATION(T);
   Dart_Handle result = Api::CheckAndFinalizePendingClasses(T);
-  if (::Dart_IsError(result)) {
+  if (Api::IsError(result)) {
     return result;
   }
   CHECK_CALLBACK_STATE(T);
@@ -205,7 +205,7 @@
   DARTSCOPE(Thread::Current());
   API_TIMELINE_DURATION(T);
   Dart_Handle result = Api::CheckAndFinalizePendingClasses(T);
-  if (::Dart_IsError(result)) {
+  if (Api::IsError(result)) {
     return result;
   }
   CHECK_CALLBACK_STATE(T);
diff --git a/runtime/vm/native_arguments.h b/runtime/vm/native_arguments.h
index 86d7be5..7c637b3 100644
--- a/runtime/vm/native_arguments.h
+++ b/runtime/vm/native_arguments.h
@@ -158,7 +158,12 @@
     return type_args.TypeAt(index);
   }
 
-  void SetReturn(const Object& value) const { *retval_ = value.raw(); }
+  RawObject** ReturnValueAddress() const { return retval_; }
+
+  void SetReturn(const Object& value) const {
+    ASSERT(thread_->execution_state() == Thread::kThreadInVM);
+    *retval_ = value.raw();
+  }
 
   RawObject* ReturnValue() const {
     // Tell MemorySanitizer the retval_ was initialized (by generated code).
@@ -250,7 +255,10 @@
   // exceedingly careful when we use it.  If there are any other side
   // effects in the statement that may cause GC, it could lead to
   // bugs.
-  void SetReturnUnsafe(RawObject* value) const { *retval_ = value; }
+  void SetReturnUnsafe(RawObject* value) const {
+    ASSERT(thread_->execution_state() == Thread::kThreadInVM);
+    *retval_ = value;
+  }
 
   // Returns true if the arguments are those of an instance function call.
   bool ToInstanceFunction() const {
diff --git a/runtime/vm/object_id_ring_test.cc b/runtime/vm/object_id_ring_test.cc
index 23593a7..23d47a6 100644
--- a/runtime/vm/object_id_ring_test.cc
+++ b/runtime/vm/object_id_ring_test.cc
@@ -128,67 +128,70 @@
       "}\n";
   Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, NULL);
   Dart_Handle result = Dart_Invoke(lib, NewString("main"), 0, NULL);
+  Dart_Handle moved_handle;
   intptr_t list_length = 0;
   EXPECT_VALID(result);
   EXPECT(!Dart_IsNull(result));
   EXPECT(Dart_IsList(result));
   EXPECT_VALID(Dart_ListLength(result, &list_length));
   EXPECT_EQ(3, list_length);
+
   Isolate* isolate = thread->isolate();
   Heap* heap = isolate->heap();
   ObjectIdRing* ring = isolate->object_id_ring();
   ObjectIdRing::LookupResult kind = ObjectIdRing::kInvalid;
-  RawObject* raw_obj = Api::UnwrapHandle(result);
-  // Located in new heap.
-  EXPECT(raw_obj->IsNewObject());
-  EXPECT_NE(Object::null(), raw_obj);
-  intptr_t raw_obj_id1 = ring->GetIdForObject(raw_obj);
-  EXPECT_EQ(0, raw_obj_id1);
-  // Get id 0 again.
-  EXPECT_EQ(raw_obj_id1, ring->GetIdForObject(raw_obj, ObjectIdRing::kReuseId));
-  // Add to ring a second time.
-  intptr_t raw_obj_id2 = ring->GetIdForObject(raw_obj);
-  EXPECT_EQ(1, raw_obj_id2);
-  // Get id 0 again.
-  EXPECT_EQ(raw_obj_id1, ring->GetIdForObject(raw_obj, ObjectIdRing::kReuseId));
-  RawObject* raw_obj1 = ring->GetObjectForId(raw_obj_id1, &kind);
-  EXPECT_EQ(ObjectIdRing::kValid, kind);
-  RawObject* raw_obj2 = ring->GetObjectForId(raw_obj_id2, &kind);
-  EXPECT_EQ(ObjectIdRing::kValid, kind);
-  EXPECT_NE(Object::null(), raw_obj1);
-  EXPECT_NE(Object::null(), raw_obj2);
-  EXPECT_EQ(RawObject::ToAddr(raw_obj), RawObject::ToAddr(raw_obj1));
-  EXPECT_EQ(RawObject::ToAddr(raw_obj), RawObject::ToAddr(raw_obj2));
+
   {
-    TransitionNativeToVM transition(thread);
+    TransitionNativeToVM to_vm(thread);
+    RawObject* raw_obj = Api::UnwrapHandle(result);
+    // Located in new heap.
+    EXPECT(raw_obj->IsNewObject());
+    EXPECT_NE(Object::null(), raw_obj);
+    intptr_t raw_obj_id1 = ring->GetIdForObject(raw_obj);
+    EXPECT_EQ(0, raw_obj_id1);
+    // Get id 0 again.
+    EXPECT_EQ(raw_obj_id1,
+              ring->GetIdForObject(raw_obj, ObjectIdRing::kReuseId));
+    // Add to ring a second time.
+    intptr_t raw_obj_id2 = ring->GetIdForObject(raw_obj);
+    EXPECT_EQ(1, raw_obj_id2);
+    // Get id 0 again.
+    EXPECT_EQ(raw_obj_id1,
+              ring->GetIdForObject(raw_obj, ObjectIdRing::kReuseId));
+    RawObject* raw_obj1 = ring->GetObjectForId(raw_obj_id1, &kind);
+    EXPECT_EQ(ObjectIdRing::kValid, kind);
+    RawObject* raw_obj2 = ring->GetObjectForId(raw_obj_id2, &kind);
+    EXPECT_EQ(ObjectIdRing::kValid, kind);
+    EXPECT_NE(Object::null(), raw_obj1);
+    EXPECT_NE(Object::null(), raw_obj2);
+    EXPECT_EQ(RawObject::ToAddr(raw_obj), RawObject::ToAddr(raw_obj1));
+    EXPECT_EQ(RawObject::ToAddr(raw_obj), RawObject::ToAddr(raw_obj2));
     // Force a scavenge.
     heap->CollectGarbage(Heap::kNew);
-  }
-  RawObject* raw_object_moved1 = ring->GetObjectForId(raw_obj_id1, &kind);
-  EXPECT_EQ(ObjectIdRing::kValid, kind);
-  RawObject* raw_object_moved2 = ring->GetObjectForId(raw_obj_id2, &kind);
-  EXPECT_EQ(ObjectIdRing::kValid, kind);
-  EXPECT_NE(Object::null(), raw_object_moved1);
-  EXPECT_NE(Object::null(), raw_object_moved2);
-  EXPECT_EQ(RawObject::ToAddr(raw_object_moved1),
-            RawObject::ToAddr(raw_object_moved2));
-  // Test that objects have moved.
-  EXPECT_NE(RawObject::ToAddr(raw_obj1), RawObject::ToAddr(raw_object_moved1));
-  EXPECT_NE(RawObject::ToAddr(raw_obj2), RawObject::ToAddr(raw_object_moved2));
-  // Test that we still point at the same list.
-  Dart_Handle moved_handle;
-  {
-    TransitionNativeToVM transition(thread);
+    RawObject* raw_object_moved1 = ring->GetObjectForId(raw_obj_id1, &kind);
+    EXPECT_EQ(ObjectIdRing::kValid, kind);
+    RawObject* raw_object_moved2 = ring->GetObjectForId(raw_obj_id2, &kind);
+    EXPECT_EQ(ObjectIdRing::kValid, kind);
+    EXPECT_NE(Object::null(), raw_object_moved1);
+    EXPECT_NE(Object::null(), raw_object_moved2);
+    EXPECT_EQ(RawObject::ToAddr(raw_object_moved1),
+              RawObject::ToAddr(raw_object_moved2));
+    // Test that objects have moved.
+    EXPECT_NE(RawObject::ToAddr(raw_obj1),
+              RawObject::ToAddr(raw_object_moved1));
+    EXPECT_NE(RawObject::ToAddr(raw_obj2),
+              RawObject::ToAddr(raw_object_moved2));
+    // Test that we still point at the same list.
     moved_handle = Api::NewHandle(thread, raw_object_moved1);
+    // Test id reuse.
+    EXPECT_EQ(raw_obj_id1,
+              ring->GetIdForObject(raw_object_moved1, ObjectIdRing::kReuseId));
   }
   EXPECT_VALID(moved_handle);
   EXPECT(!Dart_IsNull(moved_handle));
   EXPECT(Dart_IsList(moved_handle));
   EXPECT_VALID(Dart_ListLength(moved_handle, &list_length));
   EXPECT_EQ(3, list_length);
-  // Test id reuse.
-  EXPECT_EQ(raw_obj_id1,
-            ring->GetIdForObject(raw_object_moved1, ObjectIdRing::kReuseId));
 }
 
 // Test that the ring table is updated with nulls when the old GC collects.
diff --git a/runtime/vm/service_test.cc b/runtime/vm/service_test.cc
index 03c96f2..aeb614e 100644
--- a/runtime/vm/service_test.cc
+++ b/runtime/vm/service_test.cc
@@ -243,11 +243,12 @@
     TransitionVMToNative transition(thread);
     lib = TestCase::LoadTestScript(kScript, NULL);
     EXPECT_VALID(lib);
-    vmlib ^= Api::UnwrapHandle(lib);
-    EXPECT(!vmlib.IsNull());
+    EXPECT(!Dart_IsNull(lib));
     Dart_Handle result = Dart_Invoke(lib, NewString("main"), 0, NULL);
     EXPECT_VALID(result);
   }
+  vmlib ^= Api::UnwrapHandle(lib);
+  EXPECT(!vmlib.IsNull());
   const Class& class_a = Class::Handle(GetClass(vmlib, "A"));
   EXPECT(!class_a.IsNull());
   const Function& function_c = Function::Handle(GetFunction(class_a, "c"));
@@ -264,9 +265,9 @@
   ServiceTestMessageHandler handler;
   Dart_Port port_id = PortMap::CreatePort(&handler);
   Dart_Handle port = Api::NewHandle(thread, SendPort::New(port_id));
-  EXPECT_VALID(port);
   {
     TransitionVMToNative transition(thread);
+    EXPECT_VALID(port);
     EXPECT_VALID(Dart_SetField(lib, NewString("port"), port));
   }
 
@@ -367,11 +368,12 @@
     TransitionVMToNative transition(thread);
     lib = TestCase::LoadTestScript(kScript, NULL);
     EXPECT_VALID(lib);
-    vmlib ^= Api::UnwrapHandle(lib);
-    EXPECT(!vmlib.IsNull());
+    EXPECT(!Dart_IsNull(lib));
     Dart_Handle result = Dart_Invoke(lib, NewString("main"), 0, NULL);
     EXPECT_VALID(result);
   }
+  vmlib ^= Api::UnwrapHandle(lib);
+  EXPECT(!vmlib.IsNull());
   const Class& class_a = Class::Handle(GetClass(vmlib, "A"));
   EXPECT(!class_a.IsNull());
   const Function& function_c = Function::Handle(GetFunction(class_a, "c"));
@@ -389,9 +391,9 @@
   ServiceTestMessageHandler handler;
   Dart_Port port_id = PortMap::CreatePort(&handler);
   Dart_Handle port = Api::NewHandle(thread, SendPort::New(port_id));
-  EXPECT_VALID(port);
   {
     TransitionVMToNative transition(thread);
+    EXPECT_VALID(port);
     EXPECT_VALID(Dart_SetField(lib, NewString("port"), port));
   }
 
@@ -437,11 +439,12 @@
     TransitionVMToNative transition(thread);
     lib = TestCase::LoadTestScript(kScript, NULL);
     EXPECT_VALID(lib);
-    vmlib ^= Api::UnwrapHandle(lib);
-    EXPECT(!vmlib.IsNull());
+    EXPECT(!Dart_IsNull(lib));
     Dart_Handle result = Dart_Invoke(lib, NewString("main"), 0, NULL);
     EXPECT_VALID(result);
   }
+  vmlib ^= Api::UnwrapHandle(lib);
+  EXPECT(!vmlib.IsNull());
   const Class& class_a = Class::Handle(GetClass(vmlib, "A"));
   EXPECT(!class_a.IsNull());
   const Function& function_c = Function::Handle(GetFunction(class_a, "c"));
@@ -459,9 +462,9 @@
   ServiceTestMessageHandler handler;
   Dart_Port port_id = PortMap::CreatePort(&handler);
   Dart_Handle port = Api::NewHandle(thread, SendPort::New(port_id));
-  EXPECT_VALID(port);
   {
     TransitionVMToNative transition(thread);
+    EXPECT_VALID(port);
     EXPECT_VALID(Dart_SetField(lib, NewString("port"), port));
   }
 
@@ -522,9 +525,9 @@
   ServiceTestMessageHandler handler;
   Dart_Port port_id = PortMap::CreatePort(&handler);
   Dart_Handle port = Api::NewHandle(thread, SendPort::New(port_id));
-  EXPECT_VALID(port);
   {
     TransitionVMToNative transition(thread);
+    EXPECT_VALID(port);
     EXPECT_VALID(Dart_SetField(lib, NewString("port"), port));
   }
 
@@ -579,9 +582,9 @@
   ServiceTestMessageHandler handler;
   Dart_Port port_id = PortMap::CreatePort(&handler);
   Dart_Handle port = Api::NewHandle(thread, SendPort::New(port_id));
-  EXPECT_VALID(port);
   {
     TransitionVMToNative transition(thread);
+    EXPECT_VALID(port);
     EXPECT_VALID(Dart_SetField(lib, NewString("port"), port));
   }
 
@@ -668,9 +671,9 @@
   ServiceTestMessageHandler handler;
   Dart_Port port_id = PortMap::CreatePort(&handler);
   Dart_Handle port = Api::NewHandle(thread, SendPort::New(port_id));
-  EXPECT_VALID(port);
   {
     TransitionVMToNative transition(thread);
+    EXPECT_VALID(port);
     EXPECT_VALID(Dart_SetField(lib, NewString("port"), port));
   }
 
@@ -713,9 +716,9 @@
   ServiceTestMessageHandler handler;
   Dart_Port port_id = PortMap::CreatePort(&handler);
   Dart_Handle port = Api::NewHandle(thread, SendPort::New(port_id));
-  EXPECT_VALID(port);
   {
     TransitionVMToNative transition(thread);
+    EXPECT_VALID(port);
     EXPECT_VALID(Dart_SetField(lib, NewString("port"), port));
   }
 
@@ -770,9 +773,9 @@
   ServiceTestMessageHandler handler;
   Dart_Port port_id = PortMap::CreatePort(&handler);
   Dart_Handle port = Api::NewHandle(thread, SendPort::New(port_id));
-  EXPECT_VALID(port);
   {
     TransitionVMToNative transition(thread);
+    EXPECT_VALID(port);
     EXPECT_VALID(Dart_SetField(lib, NewString("port"), port));
   }
 
diff --git a/runtime/vm/snapshot_test.cc b/runtime/vm/snapshot_test.cc
index 52bda42..2ca95a0 100644
--- a/runtime/vm/snapshot_test.cc
+++ b/runtime/vm/snapshot_test.cc
@@ -769,7 +769,11 @@
     StackZone zone(thread);
     HandleScope scope(thread);
 
-    EXPECT_VALID(Api::CheckAndFinalizePendingClasses(thread));
+    Dart_Handle result = Api::CheckAndFinalizePendingClasses(thread);
+    {
+      TransitionVMToNative to_native(thread);
+      EXPECT_VALID(result);
+    }
     timer1.Stop();
     OS::PrintErr("Without Snapshot: %" Pd64 "us\n", timer1.TotalElapsedTime());