> As to the problem itself, I have a clarifying questions: doesn't Blink
already have some choke point to determine whether it's "ok to call script
now"? If so that'd be a more natural place to add this handling than
TryCatch.
No, there isn't such a point. To make matters worse, some call-sites don't
check errors because they don't care.
Sorry for the ignorance but I would like to know V8's policy on termination.
1. What happens if the isolate is forcibly terminated (from another thread)
while running a script? Will an exception be thrown? Is a v8::TryCatch
catches the exception?
2. What happens if we try to run a script when the isolate has already been
terminated?
3. What happens if v8 calls blink code, the isolate is forcibly terminated
and then the control is back to v8?
On Fri, Nov 9, 2018 at 5:30 AM Adam Klein <ad...@chromium.org> wrote:
> Adding a couple more V8 folks who may have thoughts.
>
> On Thu, Nov 8, 2018 at 1:59 AM Kentaro Hara <hara...@chromium.org> wrote:
>
>> On Thu, Nov 8, 2018 at 1:10 AM Yuki Shiino <yukishi...@chromium.org>
>> wrote:
>>
>>> +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. :(
>>> }
>>>
>>
>> Hmm, I'm a bit confused.
>>
>> If Foo() rethrows the exception, why does the worker thread continue
>> execution and call Bar()? That will cause a problem even when the worker is
>> not terminated (because the worker continues execution ignoring the thrown
>> exception)...?
>>
>> Also I might want to understand how widely the problem is happening.
>> First of all, it is not realistic to handle worker's sudden termination in
>> 100% cases (unless we add checks to all V8 API calls in the Blink code
>> base). So the best thing we can do is to insert the termination check to
>> places that may call scripts (e.g., V8ScriptRunner, EventListener, Promise
>> resolve / reject etc) and reduce the crash rate. We could introduce
>> HasCaughtOrTerminated() if it helps to reduce the crash rate, but I want to
>> make sure that assumption is correct. My intuition is that it will be more
>> useful to identify places that may call scripts often and insert the
>> termination checks if missing.
>>
>
> I'd similarly like to understand how much this happens in practice.
>
>
>> 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.
>>>
>>
> As to the problem itself, I have a clarifying questions: doesn't Blink
> already have some choke point to determine whether it's "ok to call script
> now"? If so that'd be a more natural place to add this handling than
> TryCatch.
>
> - Adam
>
>
>> 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
>>>>
>>>
>>
>> --
>> Kentaro Hara, Tokyo, Japan
>>
>> --
>> 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/CABg10jx51JXHEt0H9z%3DzT%3D0GnhDf79QjhGtVxRc%3Dy-RXjHXUyw%40mail.gmail.com
>> <https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jx51JXHEt0H9z%3DzT%3D0GnhDf79QjhGtVxRc%3Dy-RXjHXUyw%40mail.gmail.com?utm_medium=email&utm_source=footer>
>> .
>>
>
--
--
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.
