Fix reload handling of unwind / stackoverflow errors

runtime/vm/isolate_reload.cc

 // Copyright (c) 2016, 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 "vm/isolate_reload.h"
 
 #include "vm/become.h"
 #include "vm/bit_vector.h"
 #include "vm/code_generator.h"
 #include "vm/compiler.h"
@@ skipping 347 matching lines @@
 
 
 IsolateReloadContext::IsolateReloadContext(Isolate* isolate,
                                            JSONStream* js)
     : zone_(Thread::Current()->zone()),
       start_time_micros_(OS::GetCurrentMonotonicMicros()),
       reload_timestamp_(OS::GetCurrentTimeMillis()),
       isolate_(isolate),
       reload_skipped_(false),
       reload_aborted_(false),
+      reload_finalized_(false),
       js_(js),
       saved_num_cids_(-1),
       saved_class_table_(NULL),
       num_saved_libs_(-1),
       instance_morphers_(zone_, 0),
       reasons_to_cancel_reload_(zone_, 0),
       cid_mapper_(),
       modified_libs_(NULL),
       script_uri_(String::null()),
       error_(Error::null()),
@@ skipping 89 matching lines @@
 
   // Ensure all functions on the stack have unoptimized code.
   EnsuredUnoptimizedCodeForStack();
   // Deoptimize all code that had optimizing decisions that are dependent on
   // assumptions from field guards or CHA or deferred library prefixes.
   // TODO(johnmccutchan): Deoptimizing dependent code here (before the reload)
   // is paranoid. This likely can be moved to the commit phase.
   DeoptimizeDependentCode();
   Checkpoint();
 
+  // WEIRD CONTROL FLOW BEGINS.
+  //
+  // The flow of execution until we return from the tag handler can be complex.
+  //
+  // On a successful load, the following will occur:
+  //   1) Tag Handler is invoked and the embedder is in control.
+  //   2) All sources and libraries are loaded.
+  //   3) Dart_FinalizeLoading is called by the embedder.
+  //   4) Dart_FinalizeLoading invokes IsolateReloadContext::FinalizeLoading
+  //      and we are temporarily back in control.
+  //      This is where we validate the reload and commit or reject.
+  //   5) Dart_FinalizeLoading invokes Dart code related to deferred libraries.
+  //   6) The tag handler returns and we move on.
+  //
+  // Even after a successful reload the Dart code invoked in (5) can result
+  // in an Unwind error or a StackOverflow error. This error will be returned
+  // by the tag handler. The tag handler can return other errors, for example,
+  // top level parse errors. We want to capture these errors while propagating
+  // the Unwind or StackOverflow errors.
   Object& result = Object::Handle(thread->zone());
   {
     TransitionVMToNative transition(thread);
     Api::Scope api_scope(thread);
 
     Dart_Handle retval =
         (I->library_tag_handler())(Dart_kScriptTag,
                                    Api::NewHandle(thread, Library::null()),
                                    Api::NewHandle(thread, root_lib_url.raw()));
     result = Api::UnwrapHandle(retval);
   }
+  //
+  // WEIRD CONTROL FLOW ENDS.
 
   BackgroundCompiler::Enable();
 
-  if (result.IsUnwindError()) {
-    // Ignore an unwind error because the isolate is dead.
-    return;
+  if (result.IsUnwindError() ||
+      (result.raw() == isolate()->object_store()->stack_overflow())) {
+    // When returning from the tag handler with an Unwind error or a
+    // StackOverflow error, propagate it and give up.
+    Exceptions::PropagateError(Error::Cast(result));
+    UNREACHABLE();
   }
 
+  // Other errors (e.g. a parse error) are captured by the reload system.
   if (result.IsError()) {
     FinalizeFailedLoad(Error::Cast(result));
   }
 }
 
 
 void IsolateReloadContext::RegisterClass(const Class& new_cls) {
   const Class& old_cls = Class::Handle(OldClassOrNull(new_cls));
   if (old_cls.IsNull()) {
     I->class_table()->Register(new_cls);
@@ skipping 18 matching lines @@
   AddClassMapping(new_cls, old_cls);
 }
 
 
 // FinalizeLoading will be called *before* Reload() returns but will not be
 // called if the embedder fails to load sources.
 void IsolateReloadContext::FinalizeLoading() {
   if (reload_skipped_) {
     return;
   }
+  ASSERT(!reload_finalized_);
   BuildLibraryMapping();
   TIR_Print("---- LOAD SUCCEEDED\n");
   if (ValidateReload()) {
     Commit();
     PostCommit();
     isolate()->set_last_reload_timestamp(reload_timestamp_);
   } else {
     ReportReasonsForCancelling();
     Rollback();
   }
   // ValidateReload mutates the direct subclass information and does
   // not remove dead subclasses.  Rebuild the direct subclass
   // information from scratch.
   RebuildDirectSubclasses();
-
   CommonFinalizeTail();
 }
 
 
 // FinalizeFailedLoad will be called *before* Reload() returns and will only
 // be called if the embedder fails to load sources.
 void IsolateReloadContext::FinalizeFailedLoad(const Error& error) {
   TIR_Print("---- LOAD FAILED, ABORTING RELOAD\n");
   AddReasonForCancelling(new Aborted(zone_, error));
   ReportReasonsForCancelling();
-  Rollback();
+  if (!reload_finalized_) {
+    Rollback();
+  }
   CommonFinalizeTail();
 }
 
 
 void IsolateReloadContext::CommonFinalizeTail() {
   ReportOnJSON(js_);
+  reload_finalized_ = true;
 }
 
 
 void IsolateReloadContext::ReportOnJSON(JSONStream* stream) {
+  // Clear the buffer.
+  stream->buffer()->Clear();
   JSONObject jsobj(stream);
   jsobj.AddProperty("type", "ReloadReport");
   jsobj.AddProperty("success", !HasReasonsForCancelling());
   {
     JSONObject details(&jsobj, "details");
     if (HasReasonsForCancelling()) {
       // Reload was rejected.
       JSONArray array(&jsobj, "notices");
       for (intptr_t i = 0; i < reasons_to_cancel_reload_.length(); i++) {
         ReasonForCancelling* reason = reasons_to_cancel_reload_.At(i);
@@ skipping 1033 matching lines @@
         ASSERT(!super_cls.IsNull());
         super_cls.AddDirectSubclass(cls);
       }
     }
   }
 }
 
 #endif  // !PRODUCT
 
 }  // namespace dart