1775025 - Ensure error propagation for MessageChannel::Send is complete

Status: RESOLVED WONTFIX

(Core :: IPC, task)

Description

[Jens Stutte [:jstutte]]

MessageChannel::Send is meant to inform the caller if the message dispatching was successful. We need to ensure that all possible error conditions we can encounter down the road are correctly propagated, otherwise callers might block, expecting answers they will never get.

Examples of missing propagation (might not be exhaustive):

1. Call to SendMessageToLink
2. mLink->SendMessage
3. node->SendUserMessage
4. NodeController::SendUserMessage

Comment 1

[Jens Stutte [:jstutte]]

Attachment: Bug 1775025 - Add error propagation from NodeController::SendUserMessage to MessageChannel::Send r?#ipc-reviewers

Comment 2

[Jens Stutte [:jstutte]]

There might be more of this inside Node::SendUserMessageInternal, in case for a second patch.

Comment 3

[Jens Stutte [:jstutte]]

Attachment: Bug 1775025 - Remove obsolet MessageChannel::Begin/StopPostponingSends feature. r?#ipc-reviewers

Comment 4

[Jens Stutte [:jstutte]]

And we might want to have a patch to update all uses of MessageLink::SendMessage.

Comment 5

[Nika Layzell [:nika] (ni? for response)]

We actually intentionally don't report deeper errors to the user for calls to MessageChannel::Send. For an actor, when sending an async message to the other side, there's no reasonable way to respond to the send failure, and in the event the other side disconnects (e.g. due to crashing or some other kind of error), the message can always be dropped, even if the method returns true. The only reason we really want to return an error there is for a reason under the user's control i.e. after the actor itself has already been destroyed, so CanSend() returns false.

We do need to report connection errors for sync messages, because the caller does have the ability to observe them failing, and needs to handle the fact that no response was received.

Reporting an error from Send could also give the misleading impression that if it returns true, the message will be delivered, which is not the case as every step of the message sending process could lead to it being dropped. If the user needs to react to the child process disappearing, they can use async returns messages and handle the error case, or also watch in ActorDestroy for the actor to be destroyed.

Comment 6

[Phabricator Automation]

Comment on attachment 9282068 [details]
Bug 1775025 - Remove obsolet MessageChannel::Begin/StopPostponingSends feature. r?#ipc-reviewers

Revision D149775 was moved to bug 1775112. Setting attachment 9282068 [details] to obsolete.