I filed a bug for the slightly counter-intuitive behavior I mentioned:
https://bugs.chromium.org/p/v8/issues/detail?id=8455
Cheers,
Yang
On Wed, Nov 14, 2018 at 9:01 AM Yang Guo <yang...@chromium.org> wrote:
> 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.
