[webkit-reviews] review denied: [Bug 25902] Need to implement WorkerContext.close() : [Attachment 30590] Implementation of WorkerContext.close() + layout test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 22 23:37:04 PDT 2009


Alexey Proskuryakov <ap at webkit.org> has denied 's request for review:
Bug 25902: Need to implement WorkerContext.close()
https://bugs.webkit.org/show_bug.cgi?id=25902

Attachment 30590: Implementation of WorkerContext.close() + layout test
https://bugs.webkit.org/attachment.cgi?id=30590&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
Looks like this has a race condition in the following situation:
1) Worker thread decides to close itself, calling stop(), and subsequently
destroying WorkerContext object, notifying the proxy, and exiting the thread
itself.
2) At the same time, a message is sent from the main thread to the worker
thread. Since it takes time to deliver a workerContextDestroyed notification,
this message will be added to deleted WorkerContext's queue.

I think that the right way to implement close() is send a custom message to the
proxy, asking it to terminate the context in response, and to wait for this in
some "death row" state that prevents any other communication with the proxy.

Please add a test for this situation (if at all possible, one that would crash
with this patch). Please also add a test for a situation where the worker
decides to close itself from an XHR onreadystatechange callback, not in
response to a message from the main thread. It would be nice to verify that
this doesn't introduce leaks.


More information about the webkit-reviews mailing list