Hide Isolate pointer from embedder

runtime/vm/dart_api_impl.cc

 // Copyright (c) 2013, 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 "include/dart_api.h"
 #include "include/dart_mirrors_api.h"
 #include "include/dart_native_api.h"
 
 #include "platform/assert.h"
 #include "vm/class_finalizer.h"
@@ skipping 273 matching lines @@
   if (function.IsNull()) {
     return ApiError::New(String::Handle(String::New("")));
   }
   const Array& args = Array::Handle(Array::New(kNumArgs));
   args.SetAt(0, receiver);
   args.SetAt(1, argument);
   return DartEntry::InvokeFunction(function, args);
 }
 
 
+// Convert a |Dart_Isolate| into an |Isolate|. Returns NULL if no isolate

[Ivan Posva, 2015/05/11 04:50:15]

Why not move this helper function to where it is being used in Api::CastIsolate?
Or even just inline it into the relevant casting function?

[Cutch, 2015/05/12 03:21:28]

Done.

+// could be found.
+static Isolate* LookupIsolate(Dart_Isolate isolate) {
+  return PortMap::GetIsolate(static_cast<Dart_Port>(isolate));
+}
+
+
 WeakReferenceSetBuilder* ApiState::NewWeakReferenceSetBuilder() {
   return new WeakReferenceSetBuilder(this);
 }
 
 
 void ApiState::DelayWeakReferenceSet(WeakReferenceSet* reference_set) {
   WeakReferenceSet::Push(reference_set, &delayed_weak_reference_sets_);
 }
 
 
@@ skipping 79 matching lines @@
   }
   if (ClassFinalizer::ProcessPendingClasses()) {
     return Api::Success();
   }
   ASSERT(isolate->object_store()->sticky_error() != Object::null());
   return Api::NewHandle(isolate, isolate->object_store()->sticky_error());
 }
 
 
 Dart_Isolate Api::CastIsolate(Isolate* isolate) {
-  return reinterpret_cast<Dart_Isolate>(isolate);
+  if (isolate == NULL) {
+    return ILLEGAL_ISOLATE;
+  }
+  return static_cast<Dart_Isolate>(isolate->main_port());
 }
 
 
+Isolate* Api::CastIsolate(Dart_Isolate isolate) {
+  return LookupIsolate(isolate);
+}
+
+
 Dart_Handle Api::NewError(const char* format, ...) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   CHECK_CALLBACK_STATE(isolate);
 
   va_list args;
   va_start(args, format);
   intptr_t len = OS::VSNPrint(NULL, 0, format, args);
   va_end(args);
 
@@ skipping 887 matching lines @@
     StackZone zone(isolate);
     HANDLESCOPE(isolate);
     const Error& error_obj =
         Error::Handle(isolate,
                       Dart::InitializeIsolate(snapshot, callback_data));
     if (error_obj.IsNull()) {
       if (FLAG_check_function_fingerprints) {
         Library::CheckFunctionFingerprints();
       }
       START_TIMER(isolate, time_total_runtime);
-      return reinterpret_cast<Dart_Isolate>(isolate);
+      return Api::CastIsolate(isolate);
     }
     *error = strdup(error_obj.ToErrorCString());
   }
   Dart::ShutdownIsolate();
-  return reinterpret_cast<Dart_Isolate>(NULL);
+  return ILLEGAL_ISOLATE;
 }
 
 
 DART_EXPORT void Dart_ShutdownIsolate() {
   Isolate* isolate = Isolate::Current();
   CHECK_ISOLATE(isolate);
   {
     StackZone zone(isolate);
     HandleScope handle_scope(isolate);
     Dart::RunShutdownCallback();
@@ skipping 10 matching lines @@
 
 DART_EXPORT void* Dart_CurrentIsolateData() {
   Isolate* isolate = Isolate::Current();
   CHECK_ISOLATE(isolate);
   return isolate->init_callback_data();
 }
 
 
 DART_EXPORT void* Dart_IsolateData(Dart_Isolate isolate) {
   TRACE_API_CALL(CURRENT_FUNC);
-  if (isolate == NULL) {
-    FATAL1("%s expects argument 'isolate' to be non-null.",  CURRENT_FUNC);
+  Isolate* iso = LookupIsolate(isolate);

[Ivan Posva, 2015/05/11 04:50:15]

Why not Api::CastIsolate?

[Cutch, 2015/05/12 03:21:28]

Done.

+  if (iso == NULL) {
+    FATAL1("%s expects argument 'isolate' to be a valid isolate.",
+           CURRENT_FUNC);

[Ivan Posva, 2015/05/11 04:50:15]

FATAL is a very unfriendly error from the position of the embedder. We should
evaluate our options here which would avoid killing the process.

[Cutch, 2015/05/12 03:21:29]

I've removed most uses of FATAL in this file. PTAL.

   }
-  // TODO(16615): Validate isolate parameter.
-  Isolate* iso = reinterpret_cast<Isolate*>(isolate);
   return iso->init_callback_data();
 }
 
 
 DART_EXPORT Dart_Handle Dart_DebugName() {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   return Api::NewHandle(isolate, String::New(isolate->name()));
 }
 
 
 
 DART_EXPORT void Dart_EnterIsolate(Dart_Isolate isolate) {
   CHECK_NO_ISOLATE(Isolate::Current());
-  // TODO(16615): Validate isolate parameter.
-  Isolate* iso = reinterpret_cast<Isolate*>(isolate);
+  Isolate* iso = LookupIsolate(isolate);

[Ivan Posva, 2015/05/11 04:50:15]

ditto here and below.

[Cutch, 2015/05/12 03:21:28]

Done.

+  if (iso == NULL) {
+    FATAL1("%s expects argument 'isolate' to be a valid isolate.",
+           CURRENT_FUNC);
+  }
   if (iso->mutator_thread() != NULL) {
     FATAL("Multiple mutators within one isolate is not supported.");
   }
   Thread::EnterIsolate(iso);
 }
 
 
 DART_EXPORT void Dart_IsolateBlocked() {
   Isolate* isolate = Isolate::Current();
   CHECK_ISOLATE(isolate);
@@ skipping 104 matching lines @@
   }
   ScriptSnapshotWriter writer(buffer, ApiReallocate);
   writer.WriteScriptSnapshot(library);
   *size = writer.BytesWritten();
   return Api::Success();
 }
 
 
 DART_EXPORT void Dart_InterruptIsolate(Dart_Isolate isolate) {
   TRACE_API_CALL(CURRENT_FUNC);
-  if (isolate == NULL) {
-    FATAL1("%s expects argument 'isolate' to be non-null.",  CURRENT_FUNC);
+  Isolate* iso = LookupIsolate(isolate);
+  if (iso == NULL) {
+    FATAL1("%s expects argument 'isolate' to be a valid isolate.",
+           CURRENT_FUNC);
   }
-  // TODO(16615): Validate isolate parameter.
-  Isolate* iso = reinterpret_cast<Isolate*>(isolate);
   iso->ScheduleInterrupts(Isolate::kApiInterrupt);
   // Can't use Dart_Post() since there isn't a current isolate.
   Dart_CObject api_null = { Dart_CObject_kNull , { 0 } };
   Dart_PostCObject(iso->main_port(), &api_null);
 }
 
 
 DART_EXPORT bool Dart_IsolateMakeRunnable(Dart_Isolate isolate) {
   CHECK_NO_ISOLATE(Isolate::Current());
-  if (isolate == NULL) {
-    FATAL1("%s expects argument 'isolate' to be non-null.",  CURRENT_FUNC);
+  Isolate* iso = LookupIsolate(isolate);
+  if (iso == NULL) {
+    FATAL1("%s expects argument 'isolate' to be a valid isolate.",
+           CURRENT_FUNC);
   }
-  // TODO(16615): Validate isolate parameter.
-  Isolate* iso = reinterpret_cast<Isolate*>(isolate);
   if (iso->object_store()->root_library() == Library::null()) {
     // The embedder should have called Dart_LoadScript by now.
     return false;
   }
   return iso->MakeRunnable();
 }
 
 
 // --- Messages and Ports ---
 
@@ skipping 4026 matching lines @@
     RawObject* raw_obj = obj.raw();
     isolate->heap()->SetPeer(raw_obj, peer);
   }
   return Api::Success();
 }
 
 
 // --- Service support ---
 
 DART_EXPORT bool Dart_IsServiceIsolate(Dart_Isolate isolate) {
-  Isolate* iso = reinterpret_cast<Isolate*>(isolate);
+  Isolate* iso = LookupIsolate(isolate);
   return ServiceIsolate::IsServiceIsolate(iso);
 }
 
 
 DART_EXPORT Dart_Port Dart_ServiceWaitForLoadPort() {
   return ServiceIsolate::WaitForLoadPort();
 }
 
 
 DART_EXPORT void Dart_RegisterIsolateServiceRequestCallback(
     const char* name,
     Dart_ServiceRequestCallback callback,
     void* user_data) {
   Service::RegisterIsolateEmbedderCallback(name, callback, user_data);
 }
 
 
 DART_EXPORT void Dart_RegisterRootServiceRequestCallback(
     const char* name,
     Dart_ServiceRequestCallback callback,
     void* user_data) {
   Service::RegisterRootEmbedderCallback(name, callback, user_data);
 }
 
 }  // namespace dart