Don't discard messages sent before Close() was called on a toplevel actor
Categories
(Core :: IPC, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox115 | --- | fixed |
People
(Reporter: nika, Assigned: nika, NeedInfo)
References
Details
Attachments
(1 file)
Currently, when you call Close() on a toplevel actor, we send a Goodbye message to the other side, and then close the channel connection. When that Goodbye message is received on the IO thread, the channel is marked as "closing", and any messages which have not been dispatched yet will not be run. This leads to a spew of "Channel closing: too late to send/recv, messages will be lost" logs, and means that the exact set of messages which will be discarded is racy, as the thread starting when messages are discarding is not the thread which is processing the messages.
We should continue to dispatch messages received from a process before the Goodbye message until the channel is actually notified of a closure or error on the receiving thread to make the behaviour more deterministic and useful. Any messages sent from these replies will never be delivered, but that is always the case with IPC, as the other process can disappear at any time.
|
Assignee | |
Comment 1•1 year ago
|
||
This refactoring cleans up some dead code, and makes some semantic
changes to how the MessageChannel lifecycle is handled.
These changes ensure that messages which were sent by a peer before the
GOODBYE message will be delivered, without allowing messages sent after
the GOODBYE message (e.g. by a misbehaving process) to be delivered.
The lifecycle and shutdown states were simplified, and moved to be
entirely in MessageChannel, rather than split between MessageChannel and
MessageLink.
The dead-code ChannelTimeout error state was removed, along with the
corresponding CloseWithTimeout method.
The CloseWithError method was updated to behave more consistently with
the normal Close method, synchronously triggering a connection error,
and closing the MessageLink. This method is currently unused, but will
useful in the future for handling processing errors.
Pushed by nlayzell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/03bd5f26f9af Refactor MessageChannel shutdown states, r=ipc-reviewers,mccr8
|
||
Comment 3•1 year ago
|
||
Backed out for mozilla::ThreadEventTarget::Dispatch xpcshell related crashes.
- Backout link
- Push with failures
- Failure Log
- Failure line: PROCESS-CRASH | netwerk/test/unit_ipc/test_chunked_responses_wrap.js | application crashed [@ mozilla::ThreadEventTarget::Dispatch]
Pushed by nlayzell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1e3c35a7c114 Refactor MessageChannel shutdown states, r=ipc-reviewers,mccr8
|
||
Comment 5•11 months ago
|
||
| bugherder | ||
Description
•