Make exception thrown via v8 public API propagate to v8::TryCatch as JS thrown exceptions do.

src/top.cc

 // Copyright 2006-2008 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 906 matching lines @@
   if (is_object) {
     set_pending_exception(*exception_handle);
   } else {
     // Failures are not on the heap so they neither need nor work with handles.
     ASSERT(exception_handle->IsFailure());
     set_pending_exception(exception);
   }
 }
 
 
+void Top::PropagatePendingExceptionToExteranlTryCatch() {
+  ASSERT(has_pending_exception());
+
+  if (!thread_local_.external_caught_exception_) return;
+
+  if (thread_local_.pending_exception_ == Failure::OutOfMemoryException()) {
+    // Do not propagate OOM exception: we should kill VM asap.
+  } else if (thread_local_.pending_exception_ ==
+             Heap::termination_exception()) {
+    thread_local_.TryCatchHandler()->can_continue_ = false;

[Vitaly Repeshko, 2011/01/28 14:03:33]

try_catch_handler() is a nicer alias for thread_local_.TryCatchHandler().

[antonm, 2011/01/28 14:43:18]

Done.

+    thread_local_.TryCatchHandler()->exception_ = Heap::null_value();
+  } else {
+    // At this point all non-object (failure) exceptions have
+    // been dealt with so this shouldn't fail.
+    Object* pending_exception_object = pending_exception()->ToObjectUnchecked();
+    Handle<Object> exception(pending_exception_object);

[Vitaly Repeshko, 2011/01/28 14:03:33]

Unused? If it is intended to be used, shouldn't we allocate a new handle scope
for it?

[antonm, 2011/01/28 14:43:18]

Thanks a lot, indeed unused.

+    thread_local_.TryCatchHandler()->can_continue_ = true;
+    thread_local_.TryCatchHandler()->exception_ =
+        thread_local_.pending_exception_;
+    if (!thread_local_.pending_message_obj_->IsTheHole()) {
+      try_catch_handler()->message_ = thread_local_.pending_message_obj_;
+    }
+  }
+}
+
+
 void Top::ReportPendingMessages() {

[Vitaly Repeshko, 2011/01/28 14:03:33]

It reports only a single message, doesn't it? Drop the trailing "s"?

[antonm, 2011/01/28 14:43:18]

I don't know.  Let's investigate it later.

   ASSERT(has_pending_exception());
   setup_external_caught();

[Vitaly Repeshko, 2011/01/28 14:03:33]

This looks slightly backwards: one function sets a global flag and the other
checks it. Can we convert the first one to a predicate without side effects
(IsPendingExceptionCaughtExternally) and use its return value?

[antonm, 2011/01/28 14:43:18]

I don't want to rework the things too much.  I agree that this function is weird
and even thought about getting rid of it altogether and merging it into
Propagate..., but as far as I can, I'd like to keep the change small.

+  PropagatePendingExceptionToExteranlTryCatch();
+
+  bool external_caught = thread_local_.external_caught_exception_;
   // If the pending exception is OutOfMemoryException set out_of_memory in
   // the global context.  Note: We have to mark the global context here
   // since the GenerateThrowOutOfMemory stub cannot make a RuntimeCall to
   // set it.
-  bool external_caught = thread_local_.external_caught_exception_;
-  HandleScope scope;
   if (thread_local_.pending_exception_ == Failure::OutOfMemoryException()) {
     context()->mark_out_of_memory();
   } else if (thread_local_.pending_exception_ ==
              Heap::termination_exception()) {
-    if (external_caught) {
-      thread_local_.TryCatchHandler()->can_continue_ = false;
-      thread_local_.TryCatchHandler()->exception_ = Heap::null_value();
-    }
+    // Do nothing: if needed, the exception has been already propagated to
+    // v8::TryCatch.
   } else {
-    // At this point all non-object (failure) exceptions have
-    // been dealt with so this shouldn't fail.
-    Object* pending_exception_object = pending_exception()->ToObjectUnchecked();
-    Handle<Object> exception(pending_exception_object);
-    thread_local_.external_caught_exception_ = false;
-    if (external_caught) {
-      thread_local_.TryCatchHandler()->can_continue_ = true;
-      thread_local_.TryCatchHandler()->exception_ =
-        thread_local_.pending_exception_;
-      if (!thread_local_.pending_message_obj_->IsTheHole()) {
-        try_catch_handler()->message_ = thread_local_.pending_message_obj_;
-      }
-    }
     if (thread_local_.has_pending_message_) {

[Vitaly Repeshko, 2011/01/28 14:03:33]

Join with "else" above?

[antonm, 2011/01/28 14:43:18]

I'd like to keep identical structure of failure analysis (OOM, termination,
regular exception).  But I can merge if you insist.

+      HandleScope scope;
+
+      // At this point all non-object (failure) exceptions have
+      // been dealt with so this shouldn't fail.
+      Object* pending_exception_obj = pending_exception()->ToObjectUnchecked();
+      // Message reporting executes arbitrary embedder's code and hence
+      // we need to save this state.
+      // TODO(antonm): consider saving the whole thread_local_.

[Vitaly Repeshko, 2011/01/28 14:03:33]

I don't think we should save the whole thread local state (even if it works
now). Maybe instead extract a smaller sub-object holding all the exception
related stuff?

[antonm, 2011/01/28 14:43:18]

Yep, that's why I left it as todo (there is this fancy stuff like SaveContext,
etc.)

For now I just introduced helper Scope subclass (see next patch) which stores
necessary information.

+      Handle<Object> exception(pending_exception_obj);
+      thread_local_.external_caught_exception_ = false;
+      v8::TryCatch* former_catcher = thread_local_.catcher_;
+

[Mads Ager (chromium), 2011/01/28 11:39:18]

Clear pending exception here instead of in ReportMessage? It is restored
correctly below?

[antonm, 2011/01/28 13:37:25]

See above.

[Vitaly Repeshko, 2011/01/28 14:03:33]

I agree. This logic should be all moved here.

[antonm, 2011/01/28 14:43:18]

See patch 2.

       thread_local_.has_pending_message_ = false;
       if (thread_local_.pending_message_ != NULL) {
         MessageHandler::ReportMessage(thread_local_.pending_message_);
       } else if (!thread_local_.pending_message_obj_->IsTheHole()) {
         Handle<Object> message_obj(thread_local_.pending_message_obj_);
         if (thread_local_.pending_message_script_ != NULL) {
           Handle<Script> script(thread_local_.pending_message_script_);
           int start_pos = thread_local_.pending_message_start_pos_;
           int end_pos = thread_local_.pending_message_end_pos_;
           MessageLocation location(script, start_pos, end_pos);
           MessageHandler::ReportMessage(&location, message_obj);
         } else {
           MessageHandler::ReportMessage(NULL, message_obj);
         }
       }
+
+      // Restore previously saved state.
+      thread_local_.catcher_ = former_catcher;
+      thread_local_.external_caught_exception_ = external_caught;
+      set_pending_exception(*exception);
     }
-    thread_local_.external_caught_exception_ = external_caught;
-    set_pending_exception(*exception);
   }
   clear_pending_message();
 }
 
 
 void Top::TraceException(bool flag) {
   FLAG_trace_exception = flag;
 }
 
 
 bool Top::OptionalRescheduleException(bool is_bottom_call) {
+  setup_external_caught();
+  PropagatePendingExceptionToExteranlTryCatch();
+
   // Allways reschedule out of memory exceptions.
   if (!is_out_of_memory()) {
     bool is_termination_exception =
         pending_exception() == Heap::termination_exception();
 
     // Do not reschedule the exception if this is the bottom call.
     bool clear_exception = is_bottom_call;
 
     if (is_termination_exception) {
       if (is_bottom_call) {
         thread_local_.external_caught_exception_ = false;
         clear_pending_exception();
         return false;
       }
     } else if (thread_local_.external_caught_exception_) {
       // If the exception is externally caught, clear it if there are no
       // JavaScript frames on the way to the C++ frame that has the
       // external handler.
       ASSERT(thread_local_.try_catch_handler_address() != NULL);
       Address external_handler_address =
           thread_local_.try_catch_handler_address();
       JavaScriptFrameIterator it;
       if (it.done() || (it.frame()->sp() > external_handler_address)) {

[Vitaly Repeshko, 2011/01/28 14:03:33]

Does this work correctly under ARM simulator?

[antonm, 2011/01/28 14:43:18]

I guess so, there is a special translation for simulator (the address is
actually a fake as C++ and JS stacks a different)

         clear_exception = true;
       }
     }
 
     // Clear the exception if needed.
     if (clear_exception) {
       thread_local_.external_caught_exception_ = false;
       clear_pending_exception();
       return false;
     }
@@ skipping 76 matching lines @@
 #ifdef V8_TARGET_ARCH_ARM
   thread_local_.simulator_ = Simulator::current();
 #elif V8_TARGET_ARCH_MIPS
   thread_local_.simulator_ = assembler::mips::Simulator::current();
 #endif
 #endif
   return from + sizeof(thread_local_);
 }
 
 } }  // namespace v8::internal