Closed Bug 1494469 Opened Last year Closed 5 months ago

IPC::MAX_MESSAGE_SIZE is not the maximum IPC message size

Categories

(Core :: IPC, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: jld, Assigned: hijitsuhisa)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file)

The actual maximum size for an IPC message, after serialization and headers and so on, is IPC::Channel::kMaximumMessageSize — which, at the time of this writing[1], is 256 MiB.

There is also[2] an IPC::MAX_MESSAGE_SIZE, which is 64 KiB, or 1/4096 of the actual maxmimum message size.

The history of this error seems to start with me, while doing the e10s for nsWebBrowserPersist (bug 1101100), picking 64KiB as an arbitrary block size for streaming the serialized document over IPC.  My comment, “Limit the size of an individual IPC message”, was apparently misinterpreted — I don't think it occurred to me that someone would see it and read it that way — and bug 1106321 promoted that misunderstanding to a definition in ipc/glue.

It would be nice if that could be fixed, especially because this may be causing unnecessary failures in Windows printing.

The general idea behind the 64k was that small messages mean more per-message overhead, but large messages are potentially bad for latency of other messages on the same channel (e.g., for WebBrowserPersist, that's the entire PContent tree).  However, I don't think we have any data to suggest a recommended message size.


[1] https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/ipc/chromium/src/chrome/common/ipc_channel.h#53

[2] https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/ipc/glue/IPCMessageUtils.h#111
(In reply to Jim Mathies [:jimm] from comment #1)
> We should add a comment to 
> 
> https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/ipc/glue/IPCMessageUtils.h#111

What we should do, I think, is delete that definition entirely; instead, nsWebBrowserPersist can get its old constant back, with a better comment this time, and the Windows printing thing can use something like (IPC::Channel::kMaximumMessageSize / 2).

If we really want to set out a globally recommended size for sending data chunks over IPC, there should be some actual data to justify it.

Hello,
I'm an Outreachy applicant. David Parks, the mentor for the Build A Process Sandbox Testing Framework project, sent me this bug to work on. Is there any further information about the bug I can get before starting to work on a patch ?
Thank you

Flags: needinfo?(jld)
Flags: needinfo?(jld)
Assignee: nobody → hijitsuhisa
Attachment #9053738 - Attachment description: Bug 1494469 - Removed duplicate IPC::MAX_MESSAGE_SIZE constant → Bug 1494469 - Removed duplicate IPC::MAX_MESSAGE_SIZE constant r=jld

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:hijitsuhisa, could you have a look please?

Flags: needinfo?(hijitsuhisa)
Flags: needinfo?(hijitsuhisa) → needinfo?(jld)
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e25e95c59e6b
Removed duplicate IPC::MAX_MESSAGE_SIZE constant r=jld
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Flags: needinfo?(jld)
You need to log in before you can comment on or make changes to this bug.