Large OOMs with only xul.dll frames in 36 and higher

RESOLVED FIXED in Firefox 36



4 years ago
4 years ago


(Reporter: kairo, Assigned: dmajor)



Windows NT

Firefox Tracking Flags

(firefox35 unaffected, firefox36+ fixed, firefox37+ fixed, firefox38+ fixed)


(crash signature)


(2 attachments)


Comment 1

4 years ago
xul.dll has grown by 8 megs between 34 and 36, mostly due to the DLL merges in bug 609976 and bug 922912. We likely need to update ReserveBreakpadVM's kReserveSize.

(I bet OOM|small is equally affected, but since Socorro doesn't put stack frames in those signatures, we don't notice)

Comment 2

4 years ago
Bug 1028972 comment 15: "This ought to buy us a year and change. Here's hoping we'll revisit this for unified xul before then." Well, at least that came true :-)

Comment 3

4 years ago
[Tracking Requested - why for this release]: This causes a lot of useless crash signatures on 36+
status-firefox35: --- → unaffected
status-firefox36: --- → affected
status-firefox37: --- → affected
tracking-firefox36: --- → ?
tracking-firefox37: --- → ?

Comment 4

4 years ago
Increase by 10MB to cover recent growth plus a little future breathing room.

(For anyone following along, this patch is not intended to fix the crashes, but it should turn the "xul.dll@0x1234" signatures into something more actionable)
Attachment #8553365 - Flags: review?(benjamin)
Some of the signatures I checked had lots of youtube URLs. That makes the importance of trying to figure these crashes out all the more important for MSE. Adding Anthony and Sheila to the bug as we will need to discuss this next week.


4 years ago
Attachment #8553365 - Flags: review?(benjamin) → review+
Important crash, tracking.
tracking-firefox36: ? → +
tracking-firefox37: ? → +

Comment 7

4 years ago
David, can you please request uplift to aurora and beta for this ASAP? As you know, we have those crashes on beta that we need data for and then need to react to that, so the sooner we can get this there, the better.
Flags: needinfo?(dmajor)
Comment on attachment 8553365 [details] [diff] [review]
Adjust Breakpad reservation for xul.dll inflation

[Triage Comment]
I am going to bypass the procedure to make sure we have it in beta 4.
David, hope you don't mind!
Attachment #8553365 - Flags: approval-mozilla-beta+
Attachment #8553365 - Flags: approval-mozilla-aurora+
Keywords: checkin-needed
status-firefox36: affected → fixed
status-firefox37: affected → fixed
status-firefox38: --- → fixed
tracking-firefox38: --- → ?
Keywords: checkin-needed

Comment 10

4 years ago
Given that it landed on the branches, clearing the ni?
Flags: needinfo?(dmajor)
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38

Comment 12

4 years ago
Agreed re the uplifts, thanks for jumping in!
(In reply to Robert Kaiser ( from comment #13)
> From what I see, it's substantially lower volume, but we still have reports
> like this in 36.0b4 even with this patch:

Yes, I was just going to mention that as well, shows almost all URLs which concerns me a bit since this patch made into that build ID.
tracking-firefox38: ? → +
David: Is there anything in particular we could tweak to get the remaining signatures to show more information? The concern from the MSE side would be that many URLs show youtube, and it would great to find out what the root cause of those remaining crashes is. Thanks.
Flags: needinfo?(dmajor)

Comment 16

4 years ago
The previous patch really should have worked, but I guess we can try increasing the value even higher.

In the meantime, WinDbg can make sense of the dump files if you already have a PDB cached for that xul version. If you have a list of the top N bogus signatures that you care about, I can go and open bugs with the correct stacks.
Flags: needinfo?(dmajor) → needinfo?(mozillamarcia.knous)

Comment 18

4 years ago
That one looks like it's bug 1102642.


4 years ago
Attachment #8556794 - Flags: review?(benjamin) → review+
It looks as if it may be too late to get this in B5 since I already see build candidates - ni on Sylvestre to see the possibilities.

Sylvestre we see crashes in the latest B4 explosive report that we would like to get a better stack for, so getting this patch into Beta 5 may be important for the MSE decision.
Flags: needinfo?(mozillamarcia.knous) → needinfo?(sledru)
Not sure we should postpone beta 5. Even if we start a build2 right now, we won't have the QE sign off from SV and QE before next Monday.
Beta 6 is going to build next Monday, should be released next Tuesday.
Flags: needinfo?(sledru)

Comment 22

4 years ago
Comment on attachment 8556794 [details] [diff] [review]
If at first you don't succeed, use more memory

Approval Request Comment
[Feature/regressing bug #]: xul.dll growth, mostly bug 609976 and bug 922912
[User impact if declined]: Unactionable crash reports
[Describe test coverage new/current, TreeHerder]: m-i is green
[Risks and why]: We will use more address space
[String/UUID change made/needed]: None
Attachment #8556794 - Flags: approval-mozilla-beta?
Attachment #8556794 - Flags: approval-mozilla-aurora?
Comment on attachment 8556794 [details] [diff] [review]
If at first you don't succeed, use more memory

I don't know if it is possible but maybe we should automatically detect if this needs to be increased.
Attachment #8556794 - Flags: approval-mozilla-beta?
Attachment #8556794 - Flags: approval-mozilla-beta+
Attachment #8556794 - Flags: approval-mozilla-aurora?
Attachment #8556794 - Flags: approval-mozilla-aurora+

Comment 25

4 years ago
I went ahead and pulled up the correct stacks for a few of these on 36b4: they led to bug 1128409, bug 1114976, bug 1081911.

Comment 26

4 years ago
Ryan could you help with the uplift of comment 23? (I'm surprised it didn't happen automagically. Were the scripts thrown off by the previous patch? Was there some marking I should have put on the bug?)
Flags: needinfo?(ryanvm)
(In reply to David Major [:dmajor] (UTC+13) from comment #26)
> Ryan could you help with the uplift of comment 23? (I'm surprised it didn't
> happen automagically. Were the scripts thrown off by the previous patch? Was
> there some marking I should have put on the bug?)

Yeah, the status flags should have been set back to affected when there was more follow-up work to land.
status-firefox36: fixed → affected
status-firefox37: fixed → affected
Flags: needinfo?(ryanvm)
You need to log in before you can comment on or make changes to this bug.