1767514 - Support large numbers of fds in a single message

Status: RESOLVED FIXED

(Core :: IPC, enhancement)

Description

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

Currently there is a hard limit of 200 handles attached to a single message, which generally is not a problem. This bug is tracking allowing this limit to be exceeded, by splitting up messages containing large numbers of fds into multiple sendmsg calls, and stitching them together on the receiving side.

Comment 1

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

Attachment: Bug 1767514 - Part 1: Decouple the IPC::Message max handle count and the number of FDs supported by IPC::Channel, r=#ipc-reviewers

This is done by splitting messages with large numbers of handles into multiple
sendmsg calls, each of which contains less than the maximum number of
transferred handles per-message, and stitching the message back together on the
receiving side. Most of the work on the receiving side was already handled by
the IPC::Channel code, so the work required was only to ensure we could split
the handle list up when sending.

Comment 2

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

Attachment: Bug 1767514 - Part 2: Increase the attached handle limit for IPC Messages, r=#ipc-reviewers

This is made possible by part 1, which made it possible to send more messages
using IPC::Channel. A limit is still in place, however it is now substantially
higher, hopefully making it effectively unlimited for practical purposes.

Depends on D145391

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62befea29e73
Part 1: Decouple the IPC::Message max handle count and the number of FDs supported by IPC::Channel, r=ipc-reviewers,jld
https://hg.mozilla.org/integration/autoland/rev/53ebc3f919ba
Part 2: Increase the attached handle limit for IPC Messages, r=ipc-reviewers,jld

Comment 4

[Cristian Tuns]

Backed out for causing mochitest failures on browser_fullscreen_warning.js

• Backout link
• Push with failures
• Failure Log1
• Failure Log2
• Failure Log3

Comment 5

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

[Child 1557, IPC I/O Child] WARNING: Message needs unreceived descriptors channel:1006b0040 message-type:3932222 header()->num_handles:13 num_fds:0 fds_i:0:

And the failures are all macOS, which is known to have some odd behavior around fd-passing (e.g., the sender has to hold the fds open until after they're received… or at least that used to be the case and we have code to work around it; and there was a weird edge case around almost-full socket buffers).

Comment 6

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

Poking around at this a bit, I think I've figured out that 3932222 is Msg_RegisterStringBundles on PContent (https://searchfox.org/mozilla-central/rev/c7183440fe8ad391e2ab2e990229dd85fac318d3/dom/ipc/PContent.ipdl#717). (on a side note, the message really ought to include msg->name() somewhere, but I digress). This lines up somewhat as it's a message being passed an array of FDs, which could easily have length 13. This is sent fairly early during startup as well (https://searchfox.org/mozilla-central/rev/c7183440fe8ad391e2ab2e990229dd85fac318d3/dom/ipc/ContentParent.cpp#3050), which lines up somewhat with the "WARNING: build ID mismatch false alarm" log message being emitted soon after.

It's quite surprising we'd be failing so early, as we wouldn't have had a chance to send some unexpected message with a large number of file descriptors which would need to be split into multiple sendmsg calls, so I would expect us to have actually done the exact same IPC calls now as we did previously.

I genuinely have no idea what could've changed to cause this breakage right now, but I suppose I'll come back to this later.

Comment 7

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

Attachment: Bug 1767514 - Part 3: Retry sending fds if sendmsg fails, r=#ipc-reviewers

Before this change, we wouldn't re-try sending fds if the first attempt
to send them failed, meaning that some fds wouldn't arrive if there was
any error sending (e.g. because the send buffer was full, which
is more common on macOS).

This new approach ensures we don't record that we've sent the fds until
the message is marked as successful, and should avoid the macOS errors.

Depends on D145392

Comment 8

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

I ended up finding what I think the issue is

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/344ac1cc234f
Part 1: Decouple the IPC::Message max handle count and the number of FDs supported by IPC::Channel, r=ipc-reviewers,jld
https://hg.mozilla.org/integration/autoland/rev/f3c916fcf9fe
Part 2: Increase the attached handle limit for IPC Messages, r=ipc-reviewers,jld
https://hg.mozilla.org/integration/autoland/rev/bc0432d9dd95
Part 3: Retry sending fds if sendmsg fails, r=ipc-reviewers,jld

Comment 10

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/344ac1cc234f
https://hg.mozilla.org/mozilla-central/rev/f3c916fcf9fe
https://hg.mozilla.org/mozilla-central/rev/bc0432d9dd95

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Copying crash signatures from duplicate bugs.