When you terminate execution in V8, we abort execution until the
bottom-most call into V8. If you have re-entries into V8, V8 always returns
empty results until the bottom-most call into V8. On the Blink side on the
stack of the re-entries, you can try to call into V8 before returning, but
that will simply return empty results. v8::TryCatch scopes along the way
will return true for v8::TryCatch::HasCaught and
v8::TryCatch::HasTerminated. Isolate::IsExecutionTerminating returns true.
As soon as we reach the bottom-most call, we return with an empty value as
well. The v8::TryCatch scope around that will return true for
v8::TryCatch::HasCaught and v8::TryCatch::HasTerminated, but
Isolate::IsExecutionTerminating will return false (even if you are still
inside this outer-most v8::TryCatch scope), because you can safely call
into V8 again, from here on. I actually find this a bit counter-intuitive,
and it might be better to have Isolate::IsExecutionTerminating return true,
until we leave the outer-most v8::TryCatch. Though implementing that seems
a bit annoying.
So what you are observing is that you have a non-reentry call to execute
the worker. That terminates, and you then have another non-reentry call.
That is working as intended though. Once you have left V8 through
termination across all re-entries, the isolate is good to be re-used again.
I think the correct way to fix your issue is to, at non-reentry calls, *always
check for v8::TryCatch::HasTerminated, and use that to guide the following
control flow*.
To answer your questions:
> 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?
Internally we throw a special exception that cannot be caught by
javascript. This exception causes execution to abort until we arrive at the
first (non-reentry) call into V8.
2. What happens if we try to run a script when the isolate has already been
> terminated?
If you completely exited V8 back to the first non-reentry call, you can
safely use the isolate again.
3. What happens if v8 calls blink code, the isolate is forcibly terminated and
> then the control is back to v8?
The termination exception is propagated back to V8, causes the current
execution to abort, so that we return an empty value to blink as soon as
possible. If this blink frame is called from V8, then calling into V8 will
only result in empty values.
Cheers,
Yang
On Sat, Nov 10, 2018 at 12:15 AM Adam Klein <ad...@chromium.org> wrote:
> On Thu, Nov 8, 2018 at 8:21 PM Yutaka Hirano <yhir...@chromium.org> wrote:
>
>> > 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?
>>
>>
> I'm not intimately familiar with the details here. Yang or Toon, perhaps
> you have more insight?
>
>
>>
>>
>> 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>
>>>> .
>>>>
>>> --
>> 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/CABihn6GKuTUs5UfpbQ6RwXq%3DWw0dDUJkq2JLzadnesfjLG%3DStg%40mail.gmail.com
>> <https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABihn6GKuTUs5UfpbQ6RwXq%3DWw0dDUJkq2JLzadnesfjLG%3DStg%40mail.gmail.com?utm_medium=email&utm_source=footer>
>> .
>>
> --
> 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/CAEvLGcKKKdZ4-Svz7NmXdzSaL9ryQz4cPEqhTfmWEURzwTdMqQ%40mail.gmail.com
> <https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAEvLGcKKKdZ4-Svz7NmXdzSaL9ryQz4cPEqhTfmWEURzwTdMqQ%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.
