WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
36239
[v8] Avoid reentry into v8 after TerminateExecution() on a worker thread.
https://bugs.webkit.org/show_bug.cgi?id=36239
Summary
[v8] Avoid reentry into v8 after TerminateExecution() on a worker thread.
Dmitry Titov
Reported
2010-03-17 13:03:41 PDT
V8 will crash or return Null values if used after v8::V8::TerminateExecution is invoked. We use it to terminate a worker. That exits JS stack but if the C++ code tries to execute JS again, crashes happen. For example, when worker has a MessagePort and receives a lot of messages, the MessagePort::dispatchMessages() will loop over accumulated messages and fire onmessage event. There are multiple places like that. So some sort of check for possible continuation should be made. The v8 exposes a instance-wide flag, IsExecutionTerminating(), and we have a flag in WorkerScritpController that indicates we have called TerminateExecution. I've looked over the code and realized that adding yet another check to various existing ones (valid proxy, canExecuteScripts, IsPaused etc) would only complicate things. Instead, it seems most JS execution requires to obtain a v8 context, in the form of WorkerContextExecutionProxy - and most places already check for it being 0 and bail out. So I've added a check for termination into WorkerScriptController::proxy() accessor and fixed up couple of places that didn't have check for 0. this way, we can stop v8 reentry w/o additional check sprinkled around. Patch coming.
Attachments
Patch.
(7.25 KB, patch)
2010-03-17 13:14 PDT
,
Dmitry Titov
dglazkov
: review+
dimich
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Dmitry Titov
Comment 1
2010-03-17 13:14:30 PDT
Created
attachment 50947
[details]
Patch.
WebKit Review Bot
Comment 2
2010-03-17 13:18:58 PDT
Attachment 50947
[details]
did not build on chromium: Build output:
http://webkit-commit-queue.appspot.com/results/845051
Dmitry Titov
Comment 3
2010-03-17 13:23:17 PDT
Please ignore Chromium EWS build failure - it is using older v8 revision. I'll submit updated DEPS for it before landing this patch, to avoid chromium build bot break. Separately, this patch went through chromium try bots on 3 platforms with green.
Dimitri Glazkov (Google)
Comment 4
2010-03-17 13:55:25 PDT
Comment on
attachment 50947
[details]
Patch. ok.
Dmitry Titov
Comment 5
2010-03-17 15:39:02 PDT
Landed:
http://trac.webkit.org/changeset/56131
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug