Open
Bug 974176
Opened 11 years ago
Updated 2 years ago
Windows IPC channel implementation can get stuck in ChannelOpening if a child process crashes to early
Categories
(Core :: IPC, defect, P3)
Tracking
()
NEW
People
(Reporter: bent.mozilla, Unassigned)
Details
Attachments
(1 file)
(deleted),
patch
|
benjamin
:
review-
bbondy
:
feedback+
|
Details | Diff | Splinter Review |
Bug 956218 turned the dom/ipc/tests/test_process_error.xul orange reliably on all Windows builds. The test basically launches a child process and then tells it to kill itself immediately. The PBackground test code that I added opens an IPC channel to the parent on startup, but then the child process dies before the child ever connects to the parent pipe. The Windows channel implementation doesn't notice that the child died and so the channel and its IPDL actors never get torn down properly. My code spins the event loop on shutdown waiting for all PBackground actors to clean themselves up, so the test causes us to hang on shutdown.
The code path that we go through for 'opens' protocols (like PBackground, PCompositor, PImageBridge) is different from the code path that we go through for PContent, so I don't think we have this same problem for normal process launch.
The posix IPC channel implementation seems to handle this case (at least, the test is green on all non-Windows builds), though I have no idea how.
The attached patch fixes the problem locally and on try. It works by adding a process handle watch to the channel implementation when the pipe is being opened by another process. This code doesn't really have an owner so I'm going to try seein gif bbondy wants to review it.
Attachment #8377916 -
Flags: review?(netzen)
Comment 1•11 years ago
|
||
Comment on attachment 8377916 [details] [diff] [review]
Add a process watch to ipc_channel_win, v1
Review of attachment 8377916 [details] [diff] [review]:
-----------------------------------------------------------------
This looks good to me but I'd pass it by an IPC peer
Attachment #8377916 -
Flags: review?(netzen) → feedback+
Reporter | ||
Updated•11 years ago
|
Attachment #8377916 -
Flags: review?(benjamin)
Reporter | ||
Comment 2•11 years ago
|
||
(In reply to Brian R. Bondy [:bbondy] from comment #1)
> This looks good to me but I'd pass it by an IPC peer
Thanks Brian!
Comment 3•11 years ago
|
||
Comment on attachment 8377916 [details] [diff] [review]
Add a process watch to ipc_channel_win, v1
I at least need some serious comments explaining why we need kProcessExitHandlerBit and how MessagePumpForIO::WaitForWork uses it to distinguish... something.
Attachment #8377916 -
Flags: review?(benjamin) → review-
Updated•7 years ago
|
Assignee: bent.mozilla → nobody
Status: ASSIGNED → NEW
Priority: -- → P3
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•