Closed Bug 1732343 Opened 4 months ago Closed 3 months ago

Use ports to support relaying messages with HANDLE attachments via the broker process

Categories

(Core :: IPC, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
firefox96 --- fixed

People

(Reporter: nika, Assigned: nika)

References

(Blocks 1 open bug)

Details

Attachments

(10 files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

This should allow for improved performance over the existing strategy, as we won't need to use the broker to perform sync IPC on Windows, and should also allow us to slowly reduce the number of places where an actor needs to know the pid of the remote process ahead of time, which will allow for better ergonomics when working with actors like content bridges.

In part 2 of this patch, a large number of messages are being converted to
contain move-only types, both as direct arguments and within compound data
structures. This revealed some limitations in IPDL's handling of moveonly
types, which this patch hopes to rectify. This also required changes to allow
distinguishing between types which require move to send vs. them not having a
move constructor.

This does not fully fix the underlying issues, but attempts to preserve
existing behaviour while improving support for the new types being added. There
should be further cleanup in the future.

This is useful for the following parts, as UniqueFileHandle is a cross-platform
type which can also be used to support transferring HANDLEs between processes.

This change requires fairly sweeping changes to existing callsites, which
previously did not require owning access to the handle types when transferring.
For the most part these changes were straightforward, but manual.

Depends on D126563

Handles which are directly attached to IPC messages will be transferred by the
parent process. This is handled either directly by the parent process (if it is
one of the participants), or by relaying the message via the parent process if
it is not. Ordering issues are avoided here thanks to support in the mojo ports
code for messages being delivered out-of-order.

The actual handle values are encoded in the message after the message payload,
and removed from the message before handing it off to existing code, so it
should be fully transparent.

In addition, a new flag is added to the message header to support marking a
message as a "relay" message, as well as support for deserializing these
messages with an extra NodeName (the real target/source) in the message footer.

Depends on D126564

Previously this code was using ipc::DuplicateHandle, which was unnecessary, as
the code is being run in the parent process which has access to perform direct
handle transfers. After this patch we'd like to remove ipc::DuplicateHandle and
rely only on attached handles for serialization, so the code is converted to
instead directly use DuplicateHandle.

Depends on D126570

This should unblock removing support for BrokerDuplicateHandle from our fork of
the chromium sandboxing code.

Depends on D126571

Attachment #9242789 - Attachment description: WIP: Bug 1732343 - Part 1: Better support for moveonly types in IPDL → Bug 1732343 - Part 1: Better support for moveonly types in IPDL, r=#ipc-reviewers
Attachment #9242790 - Attachment description: WIP: Bug 1732343 - Part 2: Migrate all uses of base::FileDescriptor to UniqueFileHandle → Bug 1732343 - Part 2: Migrate all uses of base::FileDescriptor to UniqueFileHandle, r=#ipc-reviewers
Attachment #9242791 - Attachment description: WIP: Bug 1732343 - Part 3: Support directly attaching handles to IPC messages on windows → Bug 1732343 - Part 3: Support directly attaching handles to IPC messages on windows, r=#ipc-reviewers
Attachment #9242792 - Attachment description: WIP: Bug 1732343 - Part 4: Use attached handles for Windows FileDescriptor serialization → Bug 1732343 - Part 4: Use attached handles for Windows FileDescriptor serialization, r=#ipc-reviewers
Attachment #9242793 - Attachment description: WIP: Bug 1732343 - Part 5: Use attached handles for Windows SharedMemory serialization → Bug 1732343 - Part 5: Use attached handles for Windows SharedMemory serialization, r=#ipc-reviewers
Attachment #9242794 - Attachment description: WIP: Bug 1732343 - Part 6: Use attached handles for Windows TransportDescriptor serialization → Bug 1732343 - Part 6: Use attached handles for Windows TransportDescriptor serialization, r=#ipc-reviewers
Attachment #9242795 - Attachment description: WIP: Bug 1732343 - Part 7: Use attached handles for Windows CrossProcess{Mutex,Semaphore} serialization → Bug 1732343 - Part 7: Use attached handles for Windows CrossProcess{Mutex,Semaphore} serialization, r=#ipc-reviewers
Attachment #9242796 - Attachment description: WIP: Bug 1732343 - Part 8: Use attached handles for Windows RemoteBackbufferHandles serialization → Bug 1732343 - Part 8: Use attached handles for Windows RemoteBackbufferHandles serialization, r=cmartin
Attachment #9242797 - Attachment description: WIP: Bug 1732343 - Part 9: Directly use DuplicateHandle in RemoteBackbuffer → Bug 1732343 - Part 9: Directly use DuplicateHandle in RemoteBackbuffer, r=cmartin
Attachment #9242798 - Attachment description: WIP: Bug 1732343 - Part 10: Remove the now-unused ipc::DuplicateHandle → Bug 1732343 - Part 10: Remove the now-unused ipc::DuplicateHandle, r=#ipc-reviewers
Blocks: 1734739
Blocks: 1734735
Duplicate of this bug: 1480324
Blocks: 1738734
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7a6708dc620a
Part 1: Better support for moveonly types in IPDL, r=handyman
https://hg.mozilla.org/integration/autoland/rev/1aa26657a7a6
Part 2: Migrate all uses of base::FileDescriptor to UniqueFileHandle, r=handyman
https://hg.mozilla.org/integration/autoland/rev/29eaabd9ffb3
Part 3: Support directly attaching handles to IPC messages on windows, r=handyman
https://hg.mozilla.org/integration/autoland/rev/7d1854782ca8
Part 4: Use attached handles for Windows FileDescriptor serialization, r=handyman
https://hg.mozilla.org/integration/autoland/rev/a292990b62de
Part 5: Use attached handles for Windows SharedMemory serialization, r=handyman
https://hg.mozilla.org/integration/autoland/rev/ccb259d73843
Part 6: Use attached handles for Windows TransportDescriptor serialization, r=handyman
https://hg.mozilla.org/integration/autoland/rev/a272da134c34
Part 7: Use attached handles for Windows CrossProcess{Mutex,Semaphore} serialization, r=handyman
https://hg.mozilla.org/integration/autoland/rev/ed0b4f757c4b
Part 8: Use attached handles for Windows RemoteBackbufferHandles serialization, r=cmartin
https://hg.mozilla.org/integration/autoland/rev/d30fa1e1f605
Part 9: Directly use DuplicateHandle in RemoteBackbuffer, r=cmartin
https://hg.mozilla.org/integration/autoland/rev/bba94c79f3e1
Part 10: Remove the now-unused ipc::DuplicateHandle, r=handyman
Regressions: 1739590

Backed out for causing coverage build bustages (Bug 1739590)

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 96 Branch → ---
Backout by csabou@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/0d855795667b
Backed out 10 changesets for causing coverage build bustages (Bug 1739590).
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fbd13c138a73
Part 1: Better support for moveonly types in IPDL, r=handyman
https://hg.mozilla.org/integration/autoland/rev/9c3d2d40c494
Part 2: Migrate all uses of base::FileDescriptor to UniqueFileHandle, r=handyman
https://hg.mozilla.org/integration/autoland/rev/14487a123efd
Part 3: Support directly attaching handles to IPC messages on windows, r=handyman
https://hg.mozilla.org/integration/autoland/rev/eea6deedfb4b
Part 4: Use attached handles for Windows FileDescriptor serialization, r=handyman
https://hg.mozilla.org/integration/autoland/rev/5cd1ca0bb8d3
Part 5: Use attached handles for Windows SharedMemory serialization, r=handyman
https://hg.mozilla.org/integration/autoland/rev/f31351d23658
Part 6: Use attached handles for Windows TransportDescriptor serialization, r=handyman
https://hg.mozilla.org/integration/autoland/rev/ea5dfb7ae6a3
Part 7: Use attached handles for Windows CrossProcess{Mutex,Semaphore} serialization, r=handyman
https://hg.mozilla.org/integration/autoland/rev/235e9c2fcbce
Part 8: Use attached handles for Windows RemoteBackbufferHandles serialization, r=cmartin
https://hg.mozilla.org/integration/autoland/rev/1d94da94e888
Part 9: Directly use DuplicateHandle in RemoteBackbuffer, r=cmartin
https://hg.mozilla.org/integration/autoland/rev/de0d40deb01d
Part 10: Remove the now-unused ipc::DuplicateHandle, r=handyman

(In reply to Pulsebot from comment #16)

Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fbd13c138a73
Part 1: Better support for moveonly types in IPDL, r=handyman
https://hg.mozilla.org/integration/autoland/rev/9c3d2d40c494
Part 2: Migrate all uses of base::FileDescriptor to UniqueFileHandle,
r=handyman
https://hg.mozilla.org/integration/autoland/rev/14487a123efd
Part 3: Support directly attaching handles to IPC messages on windows,
r=handyman
https://hg.mozilla.org/integration/autoland/rev/eea6deedfb4b
Part 4: Use attached handles for Windows FileDescriptor serialization,
r=handyman
https://hg.mozilla.org/integration/autoland/rev/5cd1ca0bb8d3
Part 5: Use attached handles for Windows SharedMemory serialization,
r=handyman
https://hg.mozilla.org/integration/autoland/rev/f31351d23658
Part 6: Use attached handles for Windows TransportDescriptor serialization,
r=handyman
https://hg.mozilla.org/integration/autoland/rev/ea5dfb7ae6a3
Part 7: Use attached handles for Windows CrossProcess{Mutex,Semaphore}
serialization, r=handyman
https://hg.mozilla.org/integration/autoland/rev/235e9c2fcbce
Part 8: Use attached handles for Windows RemoteBackbufferHandles
serialization, r=cmartin
https://hg.mozilla.org/integration/autoland/rev/1d94da94e888
Part 9: Directly use DuplicateHandle in RemoteBackbuffer, r=cmartin
https://hg.mozilla.org/integration/autoland/rev/de0d40deb01d
Part 10: Remove the now-unused ipc::DuplicateHandle, r=handyman

== Change summary for alert #32398 (as of Fri, 19 Nov 2021 13:40:21 GMT) ==

Improvements:

Ratio Test Platform Options Absolute values (old vs new)
4% tabswitch windows10-64-shippable-qr e10s fission stylo webrender 8.16 -> 7.81
4% tabswitch windows10-64-shippable-qr e10s fission stylo webrender 8.14 -> 7.80

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=32398

You need to log in before you can comment on or make changes to this bug.