[Webkit-unassigned] [Bug 51474] [Qt][WK2] WebProcess does not clean up on termination

Wed Dec 22 23:15:11 PST 2010

https://bugs.webkit.org/show_bug.cgi?id=51474

--- Comment #8 from Kimmo Kinnunen <kimmo.t.kinnunen at nokia.com>  2010-12-22 23:15:11 PST ---
As for fixing the existing bugs caused by implementing the crashhandler in the first place, i think this patch is good in general.

Rest is feedback on the crash handler, sorry about this..

However, I think the whole approach here is maybe not that wise. 

IMO the best fix would be to remove the whole crashhandler and have following contract for all the IPC and SHM files:
1. Web process creates them and sends the filename to UI process
2. UI process opens the file and deletes it
3. Web process would delete the file in its object destructors in case ui process didn't delete it. 

Point 3. would be mainly to maintain consistency. In practice it wouldn't be exercised at all, because the case ui process wouldn't open the file most likely mean that ui process crashed, and that should mean that web process would be killed with SIGKILL (or, it would, if it wouldn't have been modified to be killed with SIGINT instead..).

In practice this would leave stale files only when UI process crashes before deleting the file. This can be made very rare occasion, b/c first code that UI process runs is webkit code when handling the msg. Of course, it's a tradeoff, but I'd take it any day compared to non-terminating web processes (see below).

As for the concrete shortcomings of current code (crashhandler):

1) Doesn't survive SIGKILL. The web process will receive sigkill signals.

2) Runs c++ destructors from address space of the crashed program. It could as well be the same destructors that caused the crash in the first place.

3) Deletes files in crash handler. The file names exist in the memory space of crashed process. In other words, those filenames can contain whatever valuable filename, like my $HOME/.emacs, due to some stack/heap smashing bug. When coding properly, I don't actually think there's much that you are allowed to do in signal handlers. For one, I wouldn't call any Qt functions.- 

4) One of the main features of WebKit is that it is fast to kill the process. Now you are again introducing code that will swap in random pages of memory during exit. 

5) Relies on the bug introduced in http://trac.webkit.org/changeset/72077. This means that there's no guarantees that Web Process will be properly killed when UI process dies.. To ensure this, we would need again to write some daemon monitoring stale, locked up web processes.

Here's the subset of POSIX that you might be able to run from signal handlers: https://www.securecoding.cert.org/confluence/display/seccode/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers . 

However, I don't understand what you can do with the funcs if you cannot rely on the integrity of the memory.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.