[webkit-reviews] review denied: [Bug 224530] Wait until thread exits in CrossThreadTaskHandler::kill() : [Attachment 425997] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has denied Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 224530: Wait until thread exits in CrossThreadTaskHandler::kill()
https://bugs.webkit.org/show_bug.cgi?id=224530

Attachment 425997: Patch

https://bugs.webkit.org/attachment.cgi?id=425997&action=review




--- Comment #5 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 425997
  --> https://bugs.webkit.org/attachment.cgi?id=425997
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425997&action=review

> Source/WebKit/NetworkProcess/IndexedDB/WebIDBServer.cpp:425
> +    CrossThreadTaskHandler::kill();

I have several concerns about this patch:
1. The verb "kill", to me, is not really associated with "wait for remaining
tasks to be processed and wait for thread to exit"
2. The new code hangs the NetworkProcess's main thread on a background thread
that is used for database activity (such activity likely includes going to
disk). I don't think it is ever OK to hang the main thread on such operations.

I am all for making the code safe but making the code safe by hanging the main
thread does not seem like a good trade-off.