+adamk, cbruni to get more attention from V8 team.  This needs V8 team's
support.

Actually, I intended haraken's (a3) at my proposal (a), but I'm afraid that
changing an existing API HasCaught() would break backward compatibility.
So, I'm also proposing to add a new V8 API like
v8::TryCatch::HasPendingException or v8::TryCatch::HasCaughtOrTerminated or
whatever we call it.

By the way, the example code is just an example.  Usually we don't have two
TryCatch blocks next to each other, and Blink is rethrowing the exception
in most cases.  I just forgot to write rethrow in the example.  That's not
a point.  Let me revise the example.

Main thread:
    v8::Isolate* worker_isolate = ...;
    worker_isolate->TerminateExecution();  // Terminates the worker isolate.

Worker thread (worker isolate):
    Foo();  // The v8::Isolate terminates during execution of Foo.
    Bar();

where Foo and Bar are the followings.

    void Foo() {
      v8::TryCatch try_catch1(isolate);
      // Call V8 APIs.  The v8::Isolate gets terminated at this point.
      if (try_catch1.HasCaught()) {  // => true due to the termination
exception.
        // Rethrow or report the error to global error handlers.
        return;
      }
    }

    void Bar() {
      v8::TryCatch try_catch2(isolate);
      // Call V8 APIs.  V8 APIs fail because the isolate is terminating.
      if (try_catch2.HasCaught()) {  // => false because no new exception
is thrown inside |try_catch2| scope.
        // Rethrow or report the error to global error handlers.
        return;
      }
      // Blink reaches here.  :(
    }

A proposed solution looks like the following.

    v8::TryCatch try_catch(isolate);
    // ...
    if (try_catch.*HasCaughtOrTerminated*()) {
      v8::Local<v8::Exception> exception = try_catch.
*CaughtExceptionOrTerminationException*();
      // Do a job with |exception|.
      return;
    }

where HasCaughtOrTerminated returns true when the isolate is terminating
even if no exception is thrown inside the TryCatch scope, and
CaughtExceptionOrTerminationException returns a thrown exception if an
exception is thrown inside the TryCatch scope or the termination exception
if the isolate is terminating.

Cheers,
Yuki Shiino


2018年11月8日(木) 17:38 Kentaro Hara <hara...@chromium.org>:

> Thanks, I got it :)
>
> My proposal would be:
>
> (a3) Make HasCaught() return true when the isolate is terminated.
>
> I'm not sure if (a) or (a2) is a good idea because the fact that we didn't
> call ReThrow() means that we (intentionally) suppressed the exception.
> Thoughts?
>
>
>
>
> On Wed, Nov 7, 2018 at 9:51 PM Yuki Shiino <yukishi...@chromium.org>
> wrote:
>
>> Are you assuming that the worker is terminated while it's calling
>>> DoSomethingElseWithV8()? If yes, why does HasCaught() not return true? If
>>> no, what's a problem of continuing execution?
>>
>>
>> No, the worker is terminated while DoSomethingWithV8 (without "Else"),
>> and Blink continues running more V8 code at DoSomethingElseWithV8.  Two
>> pieces of code run consecutively.
>>
>> Worker thread (worker isolate):
>>     {
>>       v8::TryCatch try_catch(isolate);
>>       DoSomethingWithV8();  // The Isolate terminates here.
>>       if (try_catch.HasCaught()) {  // => true due to termination
>>         // Handle error or termination.
>>         return;
>>       }
>>     }
>>     {
>>       v8::TryCatch try_catch(isolate);
>>       DoSomethingElseWithV8();  // V8 APIs fail because the Isolate is
>> terminating.
>>       if (try_catch.HasCaught()) {  // => false because no new exception
>> is thrown inside the v8::TryCatch.
>>         return;  // Blink doesn't reach here.
>>       }
>>       // Blink reach here instead.  :(
>>     }
>>
>> IIUC, the second try_catch.HasCaught() doesn't return true because there
>> is no new exception thrown inside the v8::TryCatch scope, although V8 APIs
>> fail inside DoSomethingElseWithV8 due to a pending termination exception.
>>
>> Cheers,
>> Yuki Shiino
>>
>>
>> 2018年11月8日(木) 3:18 Kentaro Hara <hara...@chromium.org>:
>>
>>> Sorry, I'm not sure if I understand your example...
>>>
>>>
>>> On Wed, Nov 7, 2018 at 2:21 AM Yutaka Hirano <yhir...@chromium.org>
>>> wrote:
>>>
>>>> +ricea@
>>>>
>>>> On Wed, Nov 7, 2018 at 6:44 PM Yuki Shiino <yukishi...@chromium.org>
>>>> wrote:
>>>>
>>>>> Hi V8-team and platform-architecture-dev,
>>>>>
>>>>> TL;DR: We'd like to extend v8::TryCatch APIs a little.
>>>>>
>>>>> We're having an issue how to handle worker termination, i.e.
>>>>> v8::Isolate termination.  Roughly speaking, the situation is like below.
>>>>>
>>>>> Main thread:
>>>>>     v8::Isolate* worker_isolate = ...;
>>>>>     worker_isolate->TerminateExecution();  // Terminates the worker
>>>>> isolate.
>>>>>
>>>>> Worker thread (worker isolate):
>>>>>     v8::TryCatch try_catch(isolate);
>>>>>     DoSomethingWithV8();  // The Isolate terminates here.
>>>>>     if (try_catch.HasCaught()) {  // => true due to termination
>>>>>       // Handle error or termination.
>>>>>       return;
>>>>>     }
>>>>>
>>>>> No problem so far.  Worker thread MUST NOT run any V8 code no longer
>>>>> because the Isolate is terminating.  However, Blink is not perfect, and
>>>>> it's pretty tough for Blink to stop everything with 100% correctness.
>>>>> Occasionally (or rarely) Blink continues running more V8 code like below.
>>>>>
>>>>> Worker thread (worker isolate):
>>>>>     v8::TryCatch try_catch(isolate);
>>>>>     DoSomethingElseWithV8();
>>>>>     if (try_catch.HasCaught()) {  // => false because no new exception
>>>>> is thrown inside the v8::TryCatch.
>>>>>       return;  // Blink doesn't reach here.
>>>>>     }
>>>>>     // Blink reach here instead.  :(
>>>>>
>>>>> If v8::TryCatch::HasCaught() returned true, Blink would be able to
>>>>> handle it as error and Blink would work much better than now (Blink is now
>>>>> crashing).
>>>>>
>>>>
>>> Are you assuming that the worker is terminated while it's calling
>>> DoSomethingElseWithV8()? If yes, why does HasCaught() not return true? If
>>> no, what's a problem of continuing execution?
>>>
>>>
>>>
>>>>> So, proposals here are something like below (not yet so concrete).
>>>>>
>>>>> a) Make v8::TryCatch::HasCaught() return true if there already exists
>>>>> a pending exception.  (Maybe this is no good.)
>>>>> b) Make a new API like v8::TryCatch::HasPendingException() and make it
>>>>> return true.  Blink rewrites all HasCaught() to the new one.
>>>>>
>>>>> Similarly,
>>>>>
>>>>> a2) Make v8::TryCatch::Exception() return a pending exception if there
>>>>> already exists.
>>>>> b2) Make a new API like v8::TryCatch::PendingException() and make it
>>>>> return a thrown exception or pending exception in the isolate if any.
>>>>> Blink rewrites all Exception() to the new one.
>>>>>
>>>>> What do you think of the issue and proposals?
>>>>>
>>>>> Cheers,
>>>>> Yuki Shiino
>>>>>
>>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "platform-architecture-dev" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send
>>>> an email to platform-architecture-dev+unsubscr...@chromium.org.
>>>> To post to this group, send email to
>>>> platform-architecture-...@chromium.org.
>>>> To view this discussion on the web visit
>>>> https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABihn6ER8q9QABC3ROCSkm6V4w%2BDb0xEqRvjgzwBpMG9DmiG1Q%40mail.gmail.com
>>>> <https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABihn6ER8q9QABC3ROCSkm6V4w%2BDb0xEqRvjgzwBpMG9DmiG1Q%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>>> .
>>>>
>>>
>>>
>>> --
>>> Kentaro Hara, Tokyo, Japan
>>>
>>
>
> --
> Kentaro Hara, Tokyo, Japan
>

-- 
-- 
v8-users mailing list
v8-users@googlegroups.com
http://groups.google.com/group/v8-users
--- 
You received this message because you are subscribed to the Google Groups 
"v8-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-users+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to