crash [@ -[ChildView maybeRollup:] ]

RESOLVED WORKSFORME

Status

()

Core
Widget: Cocoa
--
critical
RESOLVED WORKSFORME
6 years ago
5 years ago

People

(Reporter: bz, Assigned: smichaud)

Tracking

({crash, topcrash})

unspecified
mozilla21
x86_64
Mac OS X
crash, topcrash
Points:
---

Firefox Tracking Flags

(firefox18 affected, firefox19+ wontfix, firefox20+ unaffected, firefox21 unaffected)

Details

(crash signature)

Attachments

(3 attachments, 1 obsolete attachment)

This bug was filed from the Socorro interface and is 
report bp-2fb93a36-2fc7-41c8-81b2-14d042110825 .
============================================================= 

This is a null-deref on this line:

  2759        gRollupListener->ShouldRollupOnMouseWheelEvent(&shouldRollup);

that I hit while two-finger-scrolling around on Google code search.  Could gRollupListener have become null between the line 2751 null-check and this code for some reason?
(Assignee)

Comment 1

6 years ago
> Could gRollupListener have become null between the line 2751
> null-check and this code for some reason?

I don't see how this could have happened.  So this crash stack is
probably misleading, and there's probably more going on there than
meets the eye.

There are a small number of other crashes [@ -[ChildView
maybeRollup:]], in different FF versions and on different versions of
OS X.  Most of them are null dereferences, but not all of them.

https://crash-stats.mozilla.com/query/query?product=Firefox&version=ALL%3AALL&platform=mac&range_value=1&range_unit=weeks&date=08%2F31%2F2011+15%3A20%3A37&query_search=signature&query_type=contains&query=maybeRollup&reason=&build_id=&process_type=any&hang_type=any&do_query=1

I'll look into this when I have the chance.
(Assignee)

Updated

6 years ago
Assignee: nobody → smichaud
Crash Signature: [@ -[ChildView maybeRollup:]] → [@ -[ChildView maybeRollup:] ]
Summary: crash [@ -[ChildView maybeRollup:]] → crash [@ -[ChildView maybeRollup:] ]
(Assignee)

Comment 2

6 years ago
(Following up comment #1)

>> Could gRollupListener have become null between the line 2751
>> null-check and this code for some reason?
>
> I don't see how this could have happened.

But unless the crash stack is misleading I think it *must* have
happened.

I still can't figure out why.  But while digging around I discovered
an apparent mistake earlier in -[ChildView maybeRollup:] --
gMenuRollup->GetSubmenuWidgetChain() is called twice.

Maybe fixing that will make these crashes go away.  If it doesn't, the
next thing to try is a second null check on gRollupListener.
(Assignee)

Comment 3

6 years ago
Created attachment 585799 [details] [diff] [review]
Remove extra call to GetSubmenuWidgetChain()

Neil, your patch for bug 404766 added a call to GetSubmenuWidgetChain(), instead of just replacing the existing one.  This looks like a mistake.
Attachment #585799 - Flags: review?
(Assignee)

Updated

6 years ago
Attachment #585799 - Flags: review? → review?(enndeakin)

Comment 4

6 years ago
Comment on attachment 585799 [details] [diff] [review]
Remove extra call to GetSubmenuWidgetChain()

Is gRollupListener null or just invalid? gRollupListener shouldn't be changed until at least Rollup is called near the end of the method.
Attachment #585799 - Flags: review?(enndeakin) → review+
(Assignee)

Comment 5

6 years ago
Comment on attachment 585799 [details] [diff] [review]
Remove extra call to GetSubmenuWidgetChain()

Landed on mozilla-inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/95b9cb1a8e2d
(Assignee)

Comment 6

6 years ago
> Is gRollupListener null or just invalid?

I can't reproduce this bug, so I don't know for sure.  But most of the Socorro crash logs call them null dereferences.

In a few weeks I'll check again to see if these crashes persist.  If they do I'll reopen this bug and try to decide what to do next.

These crashes aren't high volume, so fixing them isn't particularly urgent.
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/95b9cb1a8e2d
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12

Updated

6 years ago
Duplicate of this bug: 716438

Comment 9

6 years ago
I don't think this is fixed.
Assignee: smichaud → joshmoz
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [inbound]
> I don't think this is fixed.

Why not?

Comment 11

6 years ago
Created attachment 592286 [details] [diff] [review]
another null check v1.0

I don't know how this is getting to be NULL but since the crashes are still happening and we don't have a better idea I think we should try this.
Attachment #592286 - Flags: review?(bgirard)

Comment 12

6 years ago
There are a couple of reports from nightly builds after your patch in our crash database.

I'm not very confident that my patch will fix the problem, the stacks seem fishy and I can't tell you exactly why my patch would work for sure, but maybe we should consider it in case it helps.

Updated

6 years ago
Attachment #592286 - Flags: review?(bgirard) → review+

Updated

6 years ago
Attachment #592286 - Attachment is obsolete: true
Attachment #592286 - Flags: review+

Updated

6 years ago
Assignee: joshmoz → nobody
(Assignee)

Updated

6 years ago
Duplicate of this bug: 757522
I think I just hit this in bp-40d19efd-3f8c-4698-ae9d-e0d502120829. Both myself and the only other report with a comment noticed that the crash was related to a drop-down being open.

Comment 15

5 years ago
Created attachment 672269 [details] [diff] [review]
further safety null checks

I was looking at the crash reports for this again, which caused me to audit usage of gRollupListener. I found two places where we aren't quite as safely null checking as we should be. In both cases we check that gRollupListener != nullptr some time before we actually user gRollupListener - it could have changed between the check and the usage. How likely that is given the calls inbetween, I do not know.
Attachment #672269 - Flags: review?(smichaud)

Comment 16

5 years ago
None of the code inside there would change gRollupListener, except for the Rollup() call.

Comment 17

5 years ago
I would tend to agree with you, but given that this crash still exists this seems like the assumption most likely to be flawed.

Comment 18

5 years ago
Comment on attachment 672269 [details] [diff] [review]
further safety null checks

Yeah Neil - I'm going to drop the request. The more I think about it the more I find it hard to believe this would help. Do you have any theories here?
Attachment #672269 - Flags: review?(smichaud)

Comment 19

5 years ago
It's #9 top browser crasher in 18.0 and #6 in 19.0b1 on Mac OS X.

More reports at:
https://crash-stats.mozilla.com/report/list?signature=-[ChildView+maybeRollup%3A]
tracking-firefox19: --- → ?
Keywords: topcrash
Hardware: x86 → x86_64
(Assignee)

Updated

5 years ago
Duplicate of this bug: 832438
Lots of URLs at bug 832438 comment #0.
(In reply to Steven Michaud from comment #21)
> Lots of URLs at bug 832438 comment #0.

Adding qawanted to test around these URLs on a Mac (especially around dropdown boxes).

Steven - were there any recent (FF18) changes in this area that can be identified through code inspection? Sounds like the crash has spiked significantly.
Assignee: nobody → smichaud
status-firefox18: --- → affected
status-firefox19: --- → affected
tracking-firefox19: ? → +
Keywords: qawanted, steps-wanted
I tried testing some of the URLs on two different machines and so far I haven't been able to reproduce the crash. I focused on some of the shopping sites where people described in the comments changing the view to show more items, show a different alpha order, etc.
> Steven - were there any recent (FF18) changes in this area that can
> be identified through code inspection? Sounds like the crash has
> spiked significantly.

These crashes have been around forever.  I suspect they've only become
more prominent because we've fixed so many other, worse crashes.

For years I've meant to take a closer look at them, and their current
prominence gives me an excuse to do so.  I'll try to start sometime
next week (or the week after).

Comment 25

5 years ago
(In reply to Steven Michaud from comment #24)
> I suspect they've only become more prominent because we've fixed so many other, worse
> crashes.
Because also Mac OS X 10.5 has been desupported which was a cause of many crashes.
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:20.0) Gecko/20130121 Firefox/20.0
Build ID:20130121042018

Firefox crashed for me with this signatures [@ -[ChildView maybeRollup:] ] while I was changing filters in the Sort By dropdown:
Steps to reproduce:
1. Navigated to http://www.yankeecandle.com/yankee-candles
2. From the left side select different candle categories
3. While the candle category is loading,  change the Sort By filter - at some point Firefox crashed 
https://crash-stats.mozilla.com/report/index/bp-4843ea96-b9e1-4766-99aa-3f53e2130122

Tried to reproduce the same crash again using the same steps, but I got a different signature: 
[@ _ZThn184_N22nsComboboxControlFrame15GetRollupWidgetEv ] 
https://crash-stats.mozilla.com/report/index/bp-6efe5c0f-b2f6-4044-8f4c-3b9c62130122
https://crash-stats.mozilla.com/report/index/bp-cfaf2123-a5e5-4e80-8acb-46c132130122
Simona, I tried your STR (from comment #26) and couldn't reproduce the crashes.  Maybe it would help to have a fuller description of exactly what you did.  I find that the "Sort By" drop-down box often disappears before I have a chance to make a selection in it (and presumably trigger the crash).
(In reply to Steven Michaud from comment #27)
> Simona, I tried your STR (from comment #26) and couldn't reproduce the
> crashes.  Maybe it would help to have a fuller description of exactly what
> you did.  I find that the "Sort By" drop-down box often disappears before I
> have a chance to make a selection in it (and presumably trigger the crash).

Repeat steps 2 and 3 from Comment 26 multiple times in order to keep the website continuously loading. That did the trick for me.
> Repeat steps 2 and 3 from Comment 26 multiple times in order to keep
> the website continuously loading. That did the trick for me.

Not for me -- even 20-30 times in a row.

You must be doing something I'm not.  Maybe a very detailed
description of exactly what you do to crash will help.

Failing that, I may still be able to glean something from your crash
logs.  But I'm going to put that off til next week.
We're about to go to build with Beta 4. There's a good chance that this will go unfixed for FF19, unless we find a critical reason to land later in the cycle.
This is now (in the last week) the #16 topcrasher in FF 19b3 (with 9 crashes).  So its frequency seems to be dropping (I can't begin to guess why).

I'll still try to find time this week to pore over this bug's crash logs.  But it sounds like it's getting less urgent.

Comment 32

5 years ago
It's #10 browser crasher on Mac OS X in 18.0.1, #11 in 19.0b3, and #6 in 20.0a2.
If you consider bug 806820 fixed, it moves up two or three positions.
tracking-firefox20: --- → ?
(In reply to Steven Michaud from comment #29)
> > Repeat steps 2 and 3 from Comment 26 multiple times in order to keep
> > the website continuously loading. That did the trick for me.
> 
> Not for me -- even 20-30 times in a row.
> 
> You must be doing something I'm not.  Maybe a very detailed
> description of exactly what you do to crash will help.

Please see the screencast for more details: http://screencast.com/t/zWwOm6jB
(Assignee)

Updated

5 years ago
See Also: → bug 836236
Scoobidiver, where do you get your information about what the Mac topcrashers are on a particular branch or "release"?

Here's how I do it:

1) Visit https://crash-stats.mozilla.com/products/Firefox.
2) Click on "Advanced search".
3) Choose one or more versions under "Version".
4) Choose "Mac OS X" as the "Operating System".
5) Click the "Filter crash reports" button.

You get a list of the top 100 crashes, ranked by number of crashes.

I did this again just now for "Firefox 19.0b3", and "-[ChildView maybeRollup:]" is #19 in the list.
https://crash-stats.mozilla.com/query/query?product=Firefox&version=Firefox%3A19.0b3&platform=mac&range_value=1&range_unit=weeks&date=01%2F30%2F2013+16%3A58%3A20&query_search=signature&query_type=contains&query=&reason=&build_id=&process_type=any&hang_type=any&do_query=1
My list does include both crashes and "crash hangs".  Did you remove the "crash hangs" from your list?
(In reply to comment #33)

Thanks for the screencast.  It's given me more information, but still not enough.

The most interesting (and puzzling) part is where you go back and forth between clicking on different items in the Candles list and clicking on "Sort By".

Or at least it looks like you're just clicking on "Sort By", and not "changing the Sort By filter".

What's puzzling is that (in your screencast) the "Sort By" drop down box opens and closes multiple times each time you "click" on "Sort By" (or whatever it is you're doing).  When I click on "Sort By" (while I'm going back and forth between it and items in the Candles list) it never opens and closes more than once.

Please describe, in great detail, and very precisely, exactly what you do when you appear (in the screencast) to "click on" "Sort By".  Also tell me exactly what kind of pointing device you're using -- a trackpad? a mouse?  If it's a mouse, what kind?  Does the mouse (if that's what your using) have a scrollwheel, and if so are you using it?

Comment 38

5 years ago
(In reply to Steven Michaud from comment #34)
> Scoobidiver, where do you get your information about what the Mac
> topcrashers are on a particular branch or "release"?
I use that kind of query: https://crash-stats.mozilla.com/topcrasher/byos/Firefox/19.0b3/Mac%20OS%20X/7/browser
Since this is a higher volume topcrash on 20, marking for tracking.
status-firefox20: --- → affected
tracking-firefox20: ? → +
Bug 836236 is clearly closely related to this bug, if not a dup.

Digging around in its crash stacks I discovered a possible fix.  In a bit I'll post a patch and tryserver build.  Since Simona can reproduce this bug, I'll ask her to try it.
Created attachment 708337 [details] [diff] [review]
Possible fix

Here's the possible fix I mentioned in comment #40.

I've started tryserver builds on all platforms.  I'll post the Mac one here when it becomes available (in a few hours).
(Following up comment #41)

This fix is in code that was added by the patch for bug 701760.  So it can't be a fix for the "original" bug.

But the patch for bug 701760 (by itself or in combination with a later patch) might have triggered an increase in frequency in this bug's crashes.  So this patch might bring these crashes' frequency down to their original (low) level.

First, though, let's see if this patch fixes the bug for Simona.
Here's the Mac tryserver build for the patch from comment #41:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-26905392dba7/try-macosx64/firefox-21.0a1.en-US.mac.dmg

Please try this, Simona!  Let us know if this fixes your crashes (or changes their crash stacks in some way).
(In reply to Steven Michaud from comment #43)
> Here's the Mac tryserver build for the patch from comment #41:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-
> 26905392dba7/try-macosx64/firefox-21.0a1.en-US.mac.dmg
> 
> Please try this, Simona!  Let us know if this fixes your crashes (or changes
> their crash stacks in some way).

After several tries I got one crash with this build but unfortunately the report can't be opened.
https://crash-stats.mozilla.com/report/index/bp-b1ecb63d-d21b-45ed-a90f-c77632130131

I tried to reproduce the crash afterwards but I couldn't.
Simona, here's another tryserver build for you to test.  (It's from bug 813442, which is currently marked security sensitive, so access to it is restricted.)

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mpalmgren@mozilla.com-9bc209b82cb3/try-macosx64/firefox-21.0a1.en-US.mac.dmg
(In reply to Steven Michaud from comment #45)
> Simona, here's another tryserver build for you to test.  (It's from bug
> 813442, which is currently marked security sensitive, so access to it is
> restricted.)
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mpalmgren@mozilla.
> com-9bc209b82cb3/try-macosx64/firefox-21.0a1.en-US.mac.dmg

I could not crash Firefox on this tryserver build using the same steps as before.

Updated

5 years ago
Depends on: 837047
FYI: there is a testcase in bug 837047
(In reply to Steven Michaud from comment #45)
> Simona, here's another tryserver build for you to test.  (It's from bug
> 813442, which is currently marked security sensitive, so access to it is
> restricted.)
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mpalmgren@mozilla.
> com-9bc209b82cb3/try-macosx64/firefox-21.0a1.en-US.mac.dmg

With the latest tryserver build - Firefox doesn't crash when following the STR from bug 837047.
Dropping QAWANTED as our testing seems to confirm the try-server build works. Can we land these changes in time for 19b5?
Keywords: qawanted
QA Contact: simona.marcu
We've made some other changes in the FF20 timeframe that we hope will resolve these crashes. Wontfixing for FF19.
status-firefox19: affected → wontfix

Updated

5 years ago
Keywords: steps-wanted
Target Milestone: mozilla12 → mozilla21

Updated

5 years ago
Blocks: 837047
No longer depends on: 837047

Comment 52

5 years ago
Any news on landing in m-c?

Comment 53

5 years ago
The latest crashes happened in 20.0a2/20130206 for Aurora and 21.0a1/20130115 for m-c.

Updated

5 years ago
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago5 years ago
status-firefox20: affected → unaffected
status-firefox21: --- → unaffected
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.