Closed Bug 1681359 Opened 2 years ago Closed 4 months ago

Tool for sending large data buffers over IPC

Categories

(Core :: IPC, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
105 Branch
Tracking Status
firefox105 --- fixed

People

(Reporter: nika, Assigned: nika)

References

Details

Attachments

(2 files, 4 obsolete files)

If a large IPC message is sent, it causes crashes due to exceeding the maximum IPC message size. This can cause issues in some cases where user-controlled data payloads are being transferred over IPC, as they can be made to be too large. In many cases, the preferred solution would be to stream the data async, but this is sometimes not possible, such as when transferring a large structured clone buffer across IPC.

Chromium's Mojo has a BigBuffer type for some of these scenarios which can contain either a owned buffer or a shared memory region, allowing for large payloads to be sent without worrying about IPC message size limits. We may want a similar data structure for sending large messages over IPDL in Gecko.

See Also: → 1524237

I think I'm going to look into adding a BIgBuffer type which can be used in place of Shmem in many places which just want to be able to send a data buffer which may be larger than is reasonable to include inline in an IPC message.

Assignee: nobody → nika

This type is inspired by the Chromium BigBuffer type, and acts as a type which
intelligently either allocates a shared memory region or in-memory buffer based
on the size of the payload. The current threshold is 64k bytes, and it can be
somewhat cheaply transferred over IPC.

This is intended to be used in places where we currently create a basic Shmem
to transfer a potentially large block of bytes over IPC.

The IPCDataTransfer type is used to transfer Clipboard/Drag & Drop payloads
over IPC to allow them to be written to or read from the relevant system
interfaces. Previously, the system which was used was somewhat complex, and
tried to use Shmem in some cases to store buffers out of line. Now that
BigBuffer is available, it can be simplified substantially.

In addition, this change removed the memory buffer overload of GetSurfaceData,
as the only consumer was using it to immediately send the payload over IPC as a
nsCString. It was changed to instead use BigBuffer as that is more efficient
in a large buffer situation, and reduces the number of required copies.

Depends on D151851

The ShmemImage type was previously implemented using a Shmem, however due to
the usage patterns, BigBuffer is probably a better fit, and allows unifying
more code in nsContentUtils.

Depends on D151852

After the previous changes there was only one consumer left of the Shmem
version of GetSurfaceData, which could easily be changed to use BigBuffer,
removing the need for that overload.

After that consumer is removed, the interface was also simplified as the
generic logic in the implementation was no longer necessary.

Depends on D151853

Due to Shmem implementing ParamTraits, it is possible for a Shmem argument to
not be visible to the IPDL compiler as it is only serialized within an opaque
type included with using. If that happens, it would cause the construction of
the Shmem to fail on the other side, in a hard to diagnose manner. This changes
the logic to always allow any actor to manage shmems, to make it more in line
with the AllocShmem method being directly declared on IProtocol.

This specifically caused issues after this patch stack with PContent, which no
longer has any shmem arguments visible to IPDL after these changes, but still
is used as a manager for Shmems included in some messages.

Depends on D151854

Blocks: 1781129

Comment on attachment 9285521 [details]
Bug 1681359 - Part 2: Use BigBuffer for IPCDataTransfer, r=NeilDeakin

Revision D151852 was moved to bug 1781129. Setting attachment 9285521 [details] to obsolete.

Attachment #9285521 - Attachment is obsolete: true

Comment on attachment 9285522 [details]
Bug 1681359 - Part 3: Use BigBuffer for ShmemImage, r=NeilDeakin

Revision D151853 was moved to bug 1781129. Setting attachment 9285522 [details] to obsolete.

Attachment #9285522 - Attachment is obsolete: true

I'm planning to add a part 2 with some unit tests here, but haven't gotten around to it yet.

Comment on attachment 9285523 [details]
Bug 1681359 - Part 4: Remove Shmem overload of GetSurfaceData, r=NeilDeakin

Revision D151854 was moved to bug 1781129. Setting attachment 9285523 [details] to obsolete.

Attachment #9285523 - Attachment is obsolete: true

Comment on attachment 9285524 [details]
Bug 1681359 - Part 5: Fix issues caused by Shmem arguments no longer being visible to ipdl, r=#ipc-reviewers

Revision D151855 was moved to bug 1781129. Setting attachment 9285524 [details] to obsolete.

Attachment #9285524 - Attachment is obsolete: true
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1e32d22327ae
Part 1: Introduce the BigBuffer type to IPC, r=ipc-reviewers,jld
https://hg.mozilla.org/integration/autoland/rev/d2b1b72c06f0
Part 2: Basic tests for BigBuffer, r=jld
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
Blocks: 1783240
You need to log in before you can comment on or make changes to this bug.