More aggressively terminate child processes when the connection to their parent process is severed. Previously, we would dispatch to the main thread and then exit the process. This would sometimes cause the process to say alive for 10 seconds until our watchdog would forcefully terminate the process. This could happen in particular when the main thread is blocked on a synchronous IPC.
<rdar://problem/33317607>
Created attachment 322923 [details] Patch
Comment on attachment 322923 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322923&action=review > Source/WebKit/ChangeLog:12 > + process. This could happen in particular when the main thread is blocked on a synchronous IPC. Is it possible to fix this in sync IPC case? Exiting the process from a background thread increases the likelihood of permanently corrupting data that was being written out at the moment. I don't think that we have a good story for that either way, but making it more likely to happen seems like the wrong direction to go in.
Comment on attachment 322923 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=322923&action=review >> Source/WebKit/ChangeLog:12 >> + process. This could happen in particular when the main thread is blocked on a synchronous IPC. > > Is it possible to fix this in sync IPC case? > > Exiting the process from a background thread increases the likelihood of permanently corrupting data that was being written out at the moment. I don't think that we have a good story for that either way, but making it more likely to happen seems like the wrong direction to go in. > Is it possible to fix this in sync IPC case? I do not understand the question, the cases I experienced were cookie-related IPC messages (because the corresponding DOM API is synchronous). Getting rid of those synchronous IPCs is not trivial. When I discussed this with Sam, he indicated that the WebProcess should be robust to being terminated at any point and that we used to terminate it more aggressively. Maybe I should restrict the more aggressive killing to the WebProcess and not all child processes?
I was thinking of sync IPC towards UI process, which would be possible to interrupt if they were blocking (it's quite likely that they are already interrupted as necessary). Cookies are of course Networking process. > WebProcess should be robust to being terminated at any point I should, I don't know if it is, or even if there is any way to make it so. > we used to terminate it more aggressively Can you find out why we no longer do?
*It* should
> > we used to terminate it more aggressively > > Can you find out why we no longer do? I wasn't able to find a full record of terminating more aggressively or not, but I did find a motivation for not doing so: <https://trac.webkit.org/changeset/57549/webkit>. The concern was that exit-style termination publishes "LEAK: " reports due to WebCore objects not being deallocated before exit. Even so, I think the test case in Radar and the FIXMEs deleted in this patch demonstrate the need to forcefully exit child processes without waiting for main thread execution. So, I think we should resurrect this patch.
Comment on attachment 322923 [details] Patch r=me
Comment on attachment 322923 [details] Patch Rejecting attachment 322923 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 322923, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: -- saving rejects to file Source/WebKit/StorageProcess/StorageProcess.h.rej patching file Source/WebKit/WebProcess/WebProcess.cpp Hunk #1 FAILED at 662. 1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/WebProcess/WebProcess.cpp.rej patching file Source/WebKit/WebProcess/WebProcess.h Hunk #1 succeeded at 365 (offset 24 lines). Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Geoffrey Garen']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.webkit.org/results/7555571
Created attachment 339484 [details] Patch
Created attachment 339506 [details] Patch
Comment on attachment 339506 [details] Patch Clearing flags on attachment: 339506 Committed r231348: <https://trac.webkit.org/changeset/231348>
All reviewed patches have been landed. Closing bug.