Closed
Bug 1268662
Opened 9 years ago
Closed 8 years ago
OOM crash in Pickle::WriteBytes from replying to ReadPrefsArray()
Categories
(Core :: IPC, defect, P1)
Core
IPC
Tracking
()
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
(Keywords: crash, Whiteboard: btpp-active, e10st?)
Crash Data
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Assignee | ||
Updated•9 years ago
|
Blocks: 1262918
status-firefox47:
--- → affected
status-firefox48:
--- → affected
tracking-e10s:
--- → ?
Assignee | ||
Comment 1•9 years ago
|
||
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).
Flags: needinfo?(benjamin)
Assignee | ||
Comment 2•9 years ago
|
||
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.
Keywords: crash
Assignee | ||
Comment 3•9 years ago
|
||
I've started putting together a prototype of pref chunking.
Assignee: nobody → continuation
Comment 4•9 years ago
|
||
(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•9 years ago
|
||
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?
Flags: needinfo?(benjamin)
Updated•9 years ago
|
Whiteboard: btpp-active
Updated•9 years ago
|
Assignee | ||
Comment 6•9 years ago
|
||
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.
Updated•8 years ago
|
Whiteboard: btpp-active → btpp-active, e10st?
Assignee | ||
Comment 7•8 years ago
|
||
Bill's work to segment the IPC message buffer probably fixed this signature in general.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•