Hi Yang,
> I could change V8 to not clear the termination exception so that we always > stay in the terminated mode and not recover. However, from experience I > expect tons of tests to fail if I implemented this change. > By the way, what are "tons of tests"? Are they V8 tests? or Blink tests (web tests / layout tests)? or other embedder's tests? If they're not Blink tests, can we introduce a new API like Isolate::TerminateExecutionAndKeepExecutionTerminated? 2018年11月22日(木) 18:36 Yuki Shiino <yukishi...@chromium.org>: > What you could do is add a check after E1 for >>> v8::TryCatch::HasTerminated(), and schedule another >>> Isolate::TerminateExecution() if true. You would need to do this for E2 as >>> well, if you expect an E3, and so forth. >>> >> That might be possible by migrating >> ExceptionState.RethrowV8Exception(v8::Local<v8::Value>) to >> ExceptionState.RethrowV8Exception(v8::TryCatch&). yukishiino@, how hard >> would it be? >> > > Bindings is part of Blink. "after E1" means "after Blink callback exits > back to V8", I think? Then, there seems no chance for ExceptionState or > any bindings code to do it. > > Or, is it okay to call v8::Isolate::TerminateExecution() before Blink > exits back to V8? > > 2018年11月22日(木) 17:54 Yutaka Hirano <yhir...@chromium.org>: > >> On Thu, Nov 22, 2018 at 8:53 AM Kentaro Hara <hara...@chromium.org> >> wrote: >> >>> Would it be hard to make Blink immediately terminate the worker thread >>> after E1 is forcibly terminated? >>> >>> Blink is already doing that for common script execution paths e.g., >>> event listeners, call functions etc. >>> >> Script execution is scattered in the code base so it's hard to guarantee >> that. Most of them are short-running, so the problem should be rare, but it >> will confuse us when the problem happens. >> >> On Thu, Nov 22, 2018 at 5:25 PM Yang Guo <yang...@chromium.org> wrote: >> >>> I could change V8 to not clear the termination exception so that we >>> always stay in the terminated mode and not recover. However, from >>> experience I expect tons of tests to fail if I implemented this change. >>> >> I was thinking of introducing a new option, but yes, changing the default >> behavior may impact tests and users other than blink. >> >> >>> What you could do is add a check after E1 for >>> v8::TryCatch::HasTerminated(), and schedule another >>> Isolate::TerminateExecution() if true. You would need to do this for E2 as >>> well, if you expect an E3, and so forth. >>> >> That might be possible by migrating >> ExceptionState.RethrowV8Exception(v8::Local<v8::Value>) to >> ExceptionState.RethrowV8Exception(v8::TryCatch&). yukishiino@, how hard >> would it be? >> >> >> On Thu, Nov 22, 2018 at 5:25 PM Yang Guo <yang...@chromium.org> wrote: >> >>> I could change V8 to not clear the termination exception so that we >>> always stay in the terminated mode and not recover. However, from >>> experience I expect tons of tests to fail if I implemented this change. >>> >>> What you could do is add a check after E1 for >>> v8::TryCatch::HasTerminated(), and schedule another >>> Isolate::TerminateExecution() if true. You would need to do this for E2 as >>> well, if you expect an E3, and so forth. >>> >>> Cheers, >>> >>> Yang >>> >>> On Thu, Nov 22, 2018 at 8:53 AM Kentaro Hara <hara...@chromium.org> >>> wrote: >>> >>>> Would it be hard to make Blink immediately terminate the worker thread >>>> after E1 is forcibly terminated? >>>> >>>> Blink is already doing that for common script execution paths e.g., >>>> event listeners, call functions etc. >>>> >>>> >>>> On Thu, Nov 22, 2018 at 4:34 PM Yutaka Hirano <yhir...@chromium.org> >>>> wrote: >>>> >>>>> Thanks for the reply. >>>>> >>>>> As Kentaro said, Blink uses TerminateExecution only when it tries to >>>>> terminate a worker thread forcibly. The isolation will soon be disposed, >>>>> and the "recovering" functionality is actually harmful for us. >>>>> >>>>> main -----T------------ >>>>> worker --*E1**--*E2****** >>>>> >>>>> T: Call TerminateExecution with the worker isolate (on the main thread) >>>>> E1: bottom-most script evaluation (* means running) >>>>> E2: bottom-most script evaluation (* means running) >>>>> >>>>> In this case, the main thread wants to terminate the worker thread, >>>>> but it calls TerminateExecution only once so it E2 runs potentially >>>>> forever. >>>>> If there is an option that ensures that future v8 calls will fail >>>>> whenever possible and the v8::TryCatch scope around that will return >>>>> true for v8::TryCatch::HasCaught, then we'll be happy. Is it possible? >>>>> >>>>> >>>>> On Mon, Nov 19, 2018 at 7:27 PM Yutaka Hirano <yhir...@chromium.org> >>>>> wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> Let's consider the following sequence. >>>>>> >>>>>> -----------------------------> time >>>>>> >>>>>> main -----T------------ >>>>>> worker --*E1**--*E2***--- >>>>>> >>>>>> T: Call TerminateExecution with the worker isolate (on the main >>>>>> thread) >>>>>> E1: bottom-most script evaluation (* means running) >>>>>> E2: bottom-most script evaluation (* means running) >>>>>> >>>>>> In this case, E1 is terminated due to TerminateExecution, but E2 runs >>>>>> normally (i.e., may return a non-empty value) because "the isolate is >>>>>> good >>>>>> to be re-used again", right? >>>>>> >>>>>> Another question: If E1 starts running after TerminateExecution is >>>>>> called, what happens? >>>>>> >>>>>> main T----------------- >>>>>> worker --*E1**----------- >>>>>> >>>>>> T: Call TerminateExecution with the worker isolate (on the main >>>>>> thread) >>>>>> E1: bottom-most script evaluation (* means running) >>>>>> >>>>>> Will E1 be aborted due to a past TerminateExecution call, or will it >>>>>> run because "the isolate is good to be re-used again"? >>>>>> >>>>>> Thanks, >>>>>> >>>>>> >>>>>> On Mon, Nov 19, 2018 at 3:28 PM Yang Guo <yang...@chromium.org> >>>>>> wrote: >>>>>> >>>>>>> Sorry. I should have been more explicit here. My image of the stack >>>>>>> is growing bottom up. So A is the bottom-most V8 call. >>>>>>> >>>>>>> Yang >>>>>>> >>>>>>> On Mon, Nov 19, 2018 at 5:42 AM Yutaka Hirano <yhir...@chromium.org> >>>>>>> wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> I found I don't understand the direction. If there are only two >>>>>>>> levels, say blink-calls-v8(A)-calls-blink-calls-v8(B), which is the >>>>>>>> bottom-most v8 call, A or B? >>>>>>>> >>>>>>>> Thanks, >>>>>>>> >>>>>>>> On Fri, Nov 16, 2018 at 6:30 PM Yutaka Hirano <yhir...@chromium.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> Hi Yang, >>>>>>>>> >>>>>>>>> Thank you for the information! >>>>>>>>> Sorry for the late response. I will send a reply next week. >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> >>>>>>>>> >>>>>>>>> On Thu, Nov 15, 2018 at 9:33 PM Kentaro Hara <hara...@chromium.org> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> I found that at the only place Isolate::TerminateExecution is >>>>>>>>>>> called >>>>>>>>>>> <https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/workers/worker_thread.cc?type=cs&sq=package:chromium&g=0&l=415> >>>>>>>>>>> from >>>>>>>>>>> blink, V8 is not even running. That would mean that we don't have >>>>>>>>>>> to worry >>>>>>>>>>> about any of this? >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> At that place the main thread (which is not running V8) is >>>>>>>>>> calling TerminateExecution to terminate a running worker thread. The >>>>>>>>>> concern is on the worker thread. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Thu, Nov 15, 2018 at 6:51 PM Yang Guo <yang...@chromium.org> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> I found that at the only place Isolate::TerminateExecution is >>>>>>>>>>> called >>>>>>>>>>> <https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/workers/worker_thread.cc?type=cs&sq=package:chromium&g=0&l=415> >>>>>>>>>>> from >>>>>>>>>>> blink, V8 is not even running. That would mean that we don't have >>>>>>>>>>> to worry >>>>>>>>>>> about any of this? >>>>>>>>>>> >>>>>>>>>>> Cheers, >>>>>>>>>>> >>>>>>>>>>> Yang >>>>>>>>>>> >>>>>>>>>>> On Wed, Nov 14, 2018 at 10:00 AM Yang Guo <yang...@chromium.org> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> 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> >>>>>>>>>>>>>> . >>>>>>>>>>>>>> >>>>>>>>>>>>> -- >>>>>>>>>>> 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/CAFSTc_iBZUPpYMuUk3ouOe7cLBDizJFUjmcWuaAeVZfMBn4QqQ%40mail.gmail.com >>>>>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAFSTc_iBZUPpYMuUk3ouOe7cLBDizJFUjmcWuaAeVZfMBn4QqQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>>>>>> . >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> 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. >>>>>>>>>> >>>>>>>>> -- >>>>>>>> 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/CABihn6H0_3BFq5S2wv5Y8CDyW%3Dv-HjwskH1Wds6PmTWX5akE9g%40mail.gmail.com >>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABihn6H0_3BFq5S2wv5Y8CDyW%3Dv-HjwskH1Wds6PmTWX5akE9g%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/CABihn6F4PfQJML_kNgt5Et2nhfQfVBuXYqaNyHyORAGtbVbEjg%40mail.gmail.com >>>>> <https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABihn6F4PfQJML_kNgt5Et2nhfQfVBuXYqaNyHyORAGtbVbEjg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> >>>> >>>> -- >>>> 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/CABg10jz-oO0wE0ZGzdZ8%3DuXdZdj7UvR_KKG_12%2B_-67-LA_2Cw%40mail.gmail.com >>>> <https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jz-oO0wE0ZGzdZ8%3DuXdZdj7UvR_KKG_12%2B_-67-LA_2Cw%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.
