Rework service extensions to be safe

runtime/vm/isolate.cc

Index: runtime/vm/isolate.cc
diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index 1327ef24db29e36369f887c28b2f51cbaff245db..cf787fec95309c4547aeb0866d0160f4cd02727f 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -492,6 +492,15 @@ bool IsolateMessageHandler::HandleMessage(Message* message) {
     }
   }
   delete message;
+  if (success) {
+    const Object& result =
+        Object::Handle(zone, I->InvokePendingExtensionCalls());
+    if (result.IsError()) {
+      success = ProcessUnhandledException(Error::Cast(result));

[Ivan Posva, 2015/08/20 20:46:05]

We should propagate errors to the service isolate here instead of the isolate
itself. Unless we have an UnwindError of course.

[Cutch, 2015/08/21 14:18:56]

Done.

+    } else {
+      ASSERT(result.IsNull());
+    }
+  }
   return success;
 }
 
@@ -696,6 +705,7 @@ Isolate::Isolate(const Dart_IsolateFlags& api_flags)
       default_tag_(UserTag::null()),
       collected_closures_(GrowableObjectArray::null()),
       deoptimized_code_array_(GrowableObjectArray::null()),
+      pending_extension_calls_(GrowableObjectArray::null()),
       metrics_list_head_(NULL),
       compilation_allowed_(true),
       cha_(NULL),
@@ -1620,6 +1630,10 @@ void Isolate::VisitObjectPointers(ObjectPointerVisitor* visitor,
   visitor->VisitPointer(
       reinterpret_cast<RawObject**>(&deoptimized_code_array_));
 
+  // Visit the pending service extension calls.
+  visitor->VisitPointer(
+      reinterpret_cast<RawObject**>(&pending_extension_calls_));
+
   // Visit objects in the debugger.
   debugger()->VisitObjectPointers(visitor);
 
@@ -1825,6 +1839,56 @@ void Isolate::TrackDeoptimizedCode(const Code& code) {
 }
 
 
+void Isolate::set_pending_extension_calls(const GrowableObjectArray& value) {
+  pending_extension_calls_ = value.raw();
+}
+
+
+RawObject* Isolate::InvokePendingExtensionCalls() {
+  GrowableObjectArray& calls =
+      GrowableObjectArray::Handle(GetAndClearPendingExtensionCalls());
+  if (calls.IsNull()) {
+    return Object::null();
+  }
+  const Array& arguments = Array::Handle(Array::New(1, Heap::kNew));
+  Object& result = Object::Handle();
+  Instance& closure = Instance::Handle();
+  for (intptr_t i = 0; i < calls.Length(); i++) {
+    closure ^= calls.At(i);
+    ASSERT(!closure.IsNull());
+    arguments.SetAt(0, closure);
+    result = DartEntry::InvokeClosure(arguments);
+    if (result.IsError()) {
+      return result.raw();

[Ivan Posva, 2015/08/20 20:46:05]

Maybe we want to requeue the non-handled extension calls in case errors are not
fatal? Actually, errors in extension calls should never be fatal as they come in
from a side-channel.

As discussed we should send these errors back to the VM service.

[Cutch, 2015/08/21 14:18:56]

Done.

+    }
+    result = DartLibraryCalls::DrainMicrotaskQueue();
+    if (result.IsError()) {
+      return result.raw();
+    }
+  }
+  return Object::null();
+}
+
+
+RawGrowableObjectArray* Isolate::GetAndClearPendingExtensionCalls() {
+  RawGrowableObjectArray* r = pending_extension_calls_;
+  pending_extension_calls_ = GrowableObjectArray::null();
+  return r;
+}
+
+
+void Isolate::AppendExtensionCall(const Instance& closure) {
+  GrowableObjectArray& calls =
+      GrowableObjectArray::Handle(pending_extension_calls());
+  if (calls.IsNull()) {
+    calls ^= GrowableObjectArray::New(Heap::kOld);
+    ASSERT(!calls.IsNull());
+    set_pending_extension_calls(calls);
+  }
+  calls.Add(closure, Heap::kOld);
+}
+
+
 void Isolate::WakePauseEventHandler(Dart_Isolate isolate) {
   Isolate* iso = reinterpret_cast<Isolate*>(isolate);
   MonitorLocker ml(iso->pause_loop_monitor_);