Bug 36239 – [v8] Avoid reentry into v8 after TerminateExecution() on a worker thread.

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Dmitry Titov]

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.

[Dmitry Titov]

Created attachment 50947 [details]
Patch.

[WebKit Review Bot]

Attachment 50947 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/845051

[Dmitry Titov]

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 on attachment 50947 [details]
Patch.

ok.

[Dmitry Titov]

Landed: http://trac.webkit.org/changeset/56131