Use ports to support relaying messages with HANDLE attachments via the broker process
Categories
(Core :: IPC, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox96 | --- | fixed |
People
(Reporter: nika, Assigned: nika)
References
Details
Attachments
(10 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
Bug 1732343 - Part 2: Migrate all uses of base::FileDescriptor to UniqueFileHandle, r=#ipc-reviewers
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.
Assignee | ||
Comment 1•1 year ago
|
||
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.
Assignee | ||
Comment 2•1 year ago
|
||
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
Assignee | ||
Comment 3•1 year ago
|
||
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
Assignee | ||
Comment 4•1 year ago
|
||
Depends on D126565
Assignee | ||
Comment 5•1 year ago
|
||
Depends on D126566
Assignee | ||
Comment 6•1 year ago
|
||
Depends on D126567
Assignee | ||
Comment 7•1 year ago
|
||
Depends on D126568
Assignee | ||
Comment 8•1 year ago
|
||
Depends on D126569
Assignee | ||
Comment 9•1 year ago
|
||
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
Assignee | ||
Comment 10•1 year ago
|
||
This should unblock removing support for BrokerDuplicateHandle from our fork of
the chromium sandboxing code.
Depends on D126571
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Comment 12•1 year ago
|
||
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
Comment 13•1 year ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7a6708dc620a
https://hg.mozilla.org/mozilla-central/rev/1aa26657a7a6
https://hg.mozilla.org/mozilla-central/rev/29eaabd9ffb3
https://hg.mozilla.org/mozilla-central/rev/7d1854782ca8
https://hg.mozilla.org/mozilla-central/rev/a292990b62de
https://hg.mozilla.org/mozilla-central/rev/ccb259d73843
https://hg.mozilla.org/mozilla-central/rev/a272da134c34
https://hg.mozilla.org/mozilla-central/rev/ed0b4f757c4b
https://hg.mozilla.org/mozilla-central/rev/d30fa1e1f605
https://hg.mozilla.org/mozilla-central/rev/bba94c79f3e1
Comment 14•1 year ago
|
||
Backed out for causing coverage build bustages (Bug 1739590)
Comment 15•1 year ago
|
||
Backout by csabou@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/0d855795667b Backed out 10 changesets for causing coverage build bustages (Bug 1739590).
Comment 16•1 year ago
|
||
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
Comment 17•1 year ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fbd13c138a73
https://hg.mozilla.org/mozilla-central/rev/9c3d2d40c494
https://hg.mozilla.org/mozilla-central/rev/14487a123efd
https://hg.mozilla.org/mozilla-central/rev/eea6deedfb4b
https://hg.mozilla.org/mozilla-central/rev/5cd1ca0bb8d3
https://hg.mozilla.org/mozilla-central/rev/f31351d23658
https://hg.mozilla.org/mozilla-central/rev/ea5dfb7ae6a3
https://hg.mozilla.org/mozilla-central/rev/235e9c2fcbce
https://hg.mozilla.org/mozilla-central/rev/1d94da94e888
https://hg.mozilla.org/mozilla-central/rev/de0d40deb01d
Comment 18•1 year ago
|
||
(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
Description
•