[webkit-reviews] review denied: [Bug 88601] Worker tear-down can re-enter JSC during GC finalization pt. 2 : [Attachment 147149] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jun 12 17:14:49 PDT 2012
David Levin <levin at chromium.org> has denied review:
Bug 88601: Worker tear-down can re-enter JSC during GC finalization pt. 2
https://bugs.webkit.org/show_bug.cgi?id=88601
Attachment 147149: Patch
https://bugs.webkit.org/attachment.cgi?id=147149&action=review
------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=147149&action=review
Alexey is very familiar with the code since he was the original author. The
rest of us just came along and added to it (aka made it more complicated :( ).
> Source/WebCore/workers/WorkerMessagingProxy.cpp:377
> + // Executes workerObjectDestroyedInternal() on parent context's thread.
This comment is misleading because this method is executed on the Worker Object
thread already.
All this does is delay the object destruction.
I strongly believe that "m_workerObject = 0;" should be set here as the
WorkObject is done when this method returns.
> Source/WebCore/workers/WorkerMessagingProxy.cpp:378
> +
m_scriptExecutionContext->postTask(WorkerObjectDestroyedTask::create(this));
Personally I prefer using createCallbackTask which would have less boilerplate
code. (Much of the code in here predates that and in turn inspired it.)
For example see: WorkerMessagingProxy::postConsoleMessageToWorkerObject
You'd have to make WorkerMessagingProxy::workerObjectDestroyedInternal static
and take a WorkerMessagingProxy.
> Source/WebCore/workers/WorkerMessagingProxy.h:96
> + friend class WorkerObjectDestroyedTask;
Sort ideally (since ordering doesn't matter).
More information about the webkit-reviews
mailing list