901050 - Consider writing a mach_port_t IPC backend for OSX

Status: NEW

(Core :: IPC, defect, P3)

Description

[Matt Woodrow (:mattwoodrow)]

When sharing IOSurfaces between processes (what we use for WebGL and plugins) we currently mark them as global, so that they can be looked up by ID number from any process in the system.

A better solution would be to use mach ports for our IPC code, because this supports proper ownership transfer/sharing, similar to passing file descriptors via POSIX sockets.

It appears that the chromium guys wrote a prototype for this, and abandoned it because OSX 10.5 didn't support a feature that they needed (EVFILTER_MACHPORT).

They also did seem in depth performance measurements, and found them to be faster than sockets. Their initial descriptor says that the wins are negligible, but the actual numbers show them to be 150% - 1000% faster than FIFO sockets for the packet sizes that they frequently see.

Details:

http://www.chromium.org/developers/design-documents/os-x-interprocess-communication

http://code.google.com/p/chromium/issues/detail?id=5308

The remaining issue I see is passing filedescriptors across processes. Comment 4 in the chromium bug has a solution, but it might be Lion (10.7) only.

Comment 1

[Matt Woodrow (:mattwoodrow)]

As an update, SharedMemoryBasic_mach.mm has basically all the code we'd need for this, we just need to implement a ShareToProcess function for MacIOSurface using the same infrastructure.

Comment 2

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

We have supported sending mach ports over IPC for many years (we've been using mach-based shared memory since bug 1161166, and have been brokering them in normal IPC messages since bug 1734735).

I'm re-purposing this bug to track actually switching the underlying IPC mechanism to use mach ports for IPC, similar to how Chrome and WebKit do. This will be important for Gecko on iOS, as the strategy we use for transferring mach ports on desktop will not work due to the sandbox on that platform.

Comment 3

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

Attachment: Bug 901050 - Part 1: Make IPC::Channel virtual, r=#ipc-reviewers!

This change to how IPC::Channel is declared will make it more feasible for the
specific channel implementation to be selected at runtime. This patch will use
this to allow selecting the IPC::Channel backend using a pref on macOS.

Comment 4

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

Attachment: Bug 901050 - Part 2: Add mach port watching support to the IO thread, r=#ipc-reviewers!

This adds a new message_pump implementation, MessagePumpKqueue. This new
implementation is compatible with the existing MessagePumpLibevent, but adds
support for also watching mach ports.

This also disables building libevent and MessagePumpLibevent on macOS, as it is
redundant.

Depends on D251392

Comment 5

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

Attachment: Bug 901050 - Part 3: Add an IPC::ChannelMach Backend, r=#ipc-reviewers!

This adds a new IPC::Channel implementation which can be selected at runtime by
a pref, which uses mach ports for sending and receiving messages.

Some additional complexity is required outside of the channel, as the
ChannelHandle type needs to handle multiple different underlying types now.

Depends on D251393

Comment 6

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

Attachment: Bug 901050 - Part 4: Use IPC::Channel::Mode to indicate brokering status, r=#ipc-reviewers!

The mode_ member on ChannelPosix and ChannelWin was completely unused before
this change, so this patch re-purposes it to replace the existing privileged_
and accept_handles_ flags, removing some platform-specific methods from the
Channel interface. This also removes the argument being passed around in
NodeChannel::Introduction arguments.

Depends on D251394

Comment 7

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

Attachment: Bug 901050 - Part 6: Avoid checking backend pref in content, r=#ipc-reviewers!

This is done by adding a new static ChannelKind vtable, which will be selected
as the IO thread is initialized, and used for all operations in NodeController
which care about the underlying channel implementation.

In addition to num_relayed_attachments, create_raw_pipe and a new
is_valid_handle check were moved into the vtable as well, as they are
specific to a channel backend.

This required some changes to the NodeController interface to hide channel
creation better from callers.

Depends on D251640

Comment 8

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

Attachment: Bug 901050 - Part 7: Use the SEND_POSSIBLE notification for mach_msg timeouts, r=#ipc-reviewers!

This patch adds support to message_pump_kqueue for watching for SEND_POSSIBLE
and DEAD_NAME notifications for send rights from the kernel.

This is done in the message pump, rather than in the IPC::Channel directly, as
failing to process a queued DEAD_NAME notification from the kernel (e.g. due to
destroying a receive right with queued messages) can lead to a port leak. By
using a shared listener port in the message pump, this leak can be avoided.

Depends on D254419

Comment 9

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

Attachment: Bug 901050 - Part 5: Don't initialize IOThread until Prefs are available, r=#ipc-reviewers!

Before this change, we would initialize IOThread much earlier during startup, before Preferences are available to read (which depends on the XPCOM component manager). After this change, starting the IOThread is done during XPCOM component startup, meaning preferences are available.

This will allow us to customize IPC configuration at startup based on Preferences in the parent process. Content processes still cannot depend on Prefs to be available.

Depends on D251640

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0c9d93c0c2e8
https://hg.mozilla.org/integration/autoland/rev/961292f17f0c
Part 1: Make IPC::Channel virtual, r=ipc-reviewers,jld
https://github.com/mozilla-firefox/firefox/commit/1cb4ea55a246
https://hg.mozilla.org/integration/autoland/rev/cc98b10a1edb
Part 2: Add mach port watching support to the IO thread, r=ipc-reviewers,jld
https://github.com/mozilla-firefox/firefox/commit/21f9270e40f6
https://hg.mozilla.org/integration/autoland/rev/c907db0318bb
Part 3: Add an IPC::ChannelMach Backend, r=ipc-reviewers,jld
https://github.com/mozilla-firefox/firefox/commit/7added392b76
https://hg.mozilla.org/integration/autoland/rev/da8ae4de4b32
Part 4: Use IPC::Channel::Mode to indicate brokering status, r=ipc-reviewers,jld
https://github.com/mozilla-firefox/firefox/commit/8358951e9a33
https://hg.mozilla.org/integration/autoland/rev/759d79d031fc
Part 5: Don't initialize IOThread until Prefs are available, r=ipc-reviewers,mccr8
https://github.com/mozilla-firefox/firefox/commit/50659c4eb435
https://hg.mozilla.org/integration/autoland/rev/33418ed301d7
Part 6: Avoid checking backend pref in content, r=ipc-reviewers,jld

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b45a01cb538e
https://hg.mozilla.org/integration/autoland/rev/fc524e49401d
Revert "Bug 901050 - Part 6: Avoid checking backend pref in content, r=ipc-reviewers,jld" for causing build bustages @ipc_channel_mach.cc.

Comment 12

[agoloman]

Backed out for causing build bustages @ipc_channel_mach.cc.

• Backout link
• Push with failures
• Failure Log

Comment 13

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

(In reply to agoloman from comment #12)

Backed out for causing build bustages @ipc_channel_mach.cc.

• Backout link
• Push with failures
• Failure Log

Looks like a conflict with bug 1973598; see also this comment about argument-dependent lookup. Should be trivial to fix.