1268662 - OOM crash in Pickle::WriteBytes from replying to ReadPrefsArray()

Status: RESOLVED WORKSFORME

(Core :: IPC, defect, P1)

Description

[Andrew McCreight [:mccr8]]

This seems to be a fairly common parent process crash in the early results from beta 37. I think this is the parent writing all of the prefs into a message, to send as a reply to the child.

These OOM crashes are very large. One is 100MB, another two are around 200MB. I'm guessing that some users just have extremely large preferences, maybe due to an addon storing stuff in there. It also looks like I see some repeats from a single person, which makes sense. If you are 32-bit Windows, and your prefs serialize into a 200MB array, you are probably going to crash any time you try to start a content process.

While bug 1262671 should hopefully help with this, I wonder if there's anything at a higher level that we can do.
Example crash:
https://crash-stats.mozilla.com/report/index/796acd54-1dae-4417-8601-d25ed2160428

Comment 1

[Andrew McCreight [:mccr8]]

Benjamin, do you think it might be reasonable to split pref sending into multiple messages? It would be a little finicky to do, because you'd have to track the approximate size of the data you've put into the current message so far, and leave the array of pref data sitting around in the parent so you can send off chunks of it.

The only alternative I can think of, besides waiting for bug 1262671, is to just not send prefs when the keys or values are very large, but I don't have a good sense of how badly that will break things (presumably addons, though I haven't noticed any interesting addons in these crash reports).

Comment 2

[Andrew McCreight [:mccr8]]

There's this one user who keeps crashing over and over again in a 146MB prefs message:
https://crash-stats.mozilla.com/report/index/703dcb13-380f-4fdc-a883-121012160429

This isn't quite a startup crash, but it is a startup crash in the content process. Any user crashing with this probably can't load a webpage.

Comment 3

[Andrew McCreight [:mccr8]]

I've started putting together a prototype of pref chunking.

Comment 4

[Jim Mathies [:jimm]]

(In reply to Andrew McCreight [:mccr8] from comment #2)
> There's this one user who keeps crashing over and over again in a 146MB
> prefs message:
> https://crash-stats.mozilla.com/report/index/703dcb13-380f-4fdc-a883-
> 121012160429
> 
> This isn't quite a startup crash, but it is a startup crash in the content
> process. Any user crashing with this probably can't load a webpage.

I would think that refusing to sync large pref key/value combinations here would be preferable to crashing all the time. We could dump a warning to the console about it as well so that we eventually get some bug reports.

Comment 5

[Benjamin Smedberg]

We could certainly refuse to sync any prefs larger than a cutoff (say 4k), but I don't know whether this is a bunch of smaller prefs or one giant value. We have in the past set size limits on pref values to 1MB with the intention of making that much shorter.

I don't think you need to asynchronously chunk, do you? Could you just send a bunch of async messages each with 10-20 prefs each?

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: Send preferences to the child in chunks. WIP

This sort of works, but it is ugly, because of those code to estimate how much space is needed to serialize a particular pref entry.

I'm not sure if copying a PrefSetting will copy the nsCString it contains or not. It will trigger a call to nsCString, but that shares buffers using refcounting, sometimes.

Comment 7

[Andrew McCreight [:mccr8]]

Bill's work to segment the IPC message buffer probably fixed this signature in general.