Fix sticky error propagation from thread to isolate

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 "lib/stacktrace.h"
@@ skipping 1594 matching lines @@
 
 
 DART_EXPORT Dart_Handle Dart_RunLoop() {
   Thread* T = Thread::Current();
   Isolate* I = T->isolate();
   CHECK_API_SCOPE(T);
   CHECK_CALLBACK_STATE(T);
   API_TIMELINE_BEGIN_END;
   // The message handler run loop does not expect to have a current isolate
   // so we exit the isolate here and enter it again after the runloop is done.
   ::Dart_ExitIsolate();

[Ivan Posva, 2016/03/09 08:27:18]

To avoid my question from below we should structure this code as follows to make
sure the lifetime of T is clear.

Isolate* I;
{
  Thread* T = Thread::Current();
  I = T->isolate();
  ...
  ::Dart_ExitIsolate();
}
{
  Monitor monitor;
  MonitorLocker ml(&monitor);
  ...
}
::Dart_EnterIsolate(...);

[srdjan, 2016/03/09 19:10:15]

Adapted to the lifetime of stack objets:

  Isolate* I;
  {
    Thread* T = Thread::Current();
    I = T->isolate();
    CHECK_API_SCOPE(T);
    CHECK_CALLBACK_STATE(T);
  }
  API_TIMELINE_BEGIN_END;
  ::Dart_ExitIsolate();
  {
    Monitor monitor;
    MonitorLocker ml(&monitor);
.....

   {
     Monitor monitor;
     MonitorLocker ml(&monitor);
     RunLoopData data;
     data.monitor = &monitor;
     data.done = false;
     I->message_handler()->Run(
         Dart::thread_pool(),
         NULL, RunLoopDone, reinterpret_cast<uword>(&data));
     while (!data.done) {
       ml.Wait();
     }
   }
   ::Dart_EnterIsolate(Api::CastIsolate(I));
-  if (T->sticky_error() != Object::null()) {
-    Dart_Handle error = Api::NewHandle(T, T->sticky_error());
-    T->clear_sticky_error();
+  T = Thread::Current();

[siva, 2016/03/09 07:07:51]

Do we need this here since Api::NewHandle is called in the error path we could
just do:
Dart_Handle error = Api::NewHandle(Thread::Current(), I->o.....);

[Ivan Posva, 2016/03/09 08:27:18]

Why is this even necessary. How can T have changed between the call at the top
of this function and here?

[srdjan, 2016/03/09 19:10:15]

Done.

[srdjan, 2016/03/09 19:10:15]

::Dart_EnterIsolate(...)  sets new Thread::Current(); incidentally it is the
same thread as in T, since we reuse the just released thread (thread =
GetFromFreelistLocked(isolate)). That is how we were accidentally (but
correctly) passing sticky_error, and why clearing sticky_error when unscheduling
broke code. 

+  if (I->object_store()->sticky_error() != Object::null()) {
+    Dart_Handle error = Api::NewHandle(T, I->object_store()->sticky_error());
+    I->object_store()->clear_sticky_error();
     return error;
   }
   if (FLAG_print_class_table) {
     HANDLESCOPE(T);
     I->class_table()->Print();
   }
   return Api::Success();
 }
 
 
@@ skipping 4551 matching lines @@
   return Api::Success();
 }
 #endif  // DART_PRECOMPILER
 
 
 DART_EXPORT bool Dart_IsRunningPrecompiledCode() {
   return Dart::IsRunningPrecompiledCode();
 }
 
 }  // namespace dart