| Summary: | Chromium: Use V8::TerminateExecution to actually terminate workers. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Dmitry Titov <dimich> | ||||||
| Component: | WebCore JavaScript | Assignee: | Dmitry Titov <dimich> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, atwilson, commit-queue, dglazkov, eric, levin | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
|
Description
Dmitry Titov
2010-02-18 19:33:01 PST
Created attachment 49054 [details]
Patch.
This will fix the existing test fast/workers/stress-js-execution.html which is currently crashes on Chromium Mac and Linux bots and is disabled.
Comment on attachment 49054 [details] Patch. Rejecting patch 49054 from commit-queue. Failed to run "['git', 'svn', 'dcommit']" exit_code: 1 Last 500 characters of output: M WebCore/ChangeLog M WebCore/bindings/v8/WorkerContextExecutionProxy.cpp M WebCore/bindings/v8/WorkerScriptController.cpp A repository hook failed: MERGE request failed on '/repository/webkit/trunk': Commit blocked by pre-commit hook (exit code 1) with output: svnlook: Can't write to stream: Broken pipe The following ChangeLog files contain OOPS: trunk/WebCore/ChangeLog Please don't ever say "OOPS" in a ChangeLog file. at /usr/local/git/libexec/git-core/git-svn line 558 Full output: http://webkit-commit-queue.appspot.com/results/289144 Oops indeed. Set cq+ by mistake. Should commit-queue try to land patch without r+? Anyways, cq- now . Per our email discussion, I'm concerned that many of our uses of v8::TryCatch will now be incorrect once we start using this. We should double-check that with Mads before landing. (In reply to comment #3) > Oops indeed. Set cq+ by mistake. Should commit-queue try to land patch without > r+? Anyways, cq- now . Yup. We added this feature intentionally a while ago. It allows committers to queue their own patches for commit w/o requiring a second round of approval from a reviewer for instance. Adam Barth built a "land-safely" command around this (at least I think he did) which should be in webkit-patch. Comment on attachment 49054 [details]
Patch.
removing the r? to address concern brought by Drew.
Created attachment 49335 [details]
Updated patch.
Updated patch as per email discussion with Mads Ager and Drew Wilson.
The way to check that v8 has returned from JS block because of TerminateExecution() is to check TryCatch::CanContinue(). Added this check.
The v8 will unwind JS frames upon return from C++ frames because termination exception is 'sticky'.
Independently from this patch, V8 team plans to look into a generic issue of possible re-entering V8 while termination is in effect. It is unclear if we even have this scenario in today's worker code.
|