crash in -[NativeMenuItemTarget menuItemHit:]

VERIFIED FIXED in Firefox 41

Status

()

Core
Widget: Cocoa
--
critical
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: Tomcat, Assigned: smichaud)

Tracking

({crash, topcrash-mac})

unspecified
mozilla43
All
Mac OS X
crash, topcrash-mac
Points:
---

Firefox Tracking Flags

(firefox41 fixed, firefox42 fixed, firefox43 fixed)

Details

(crash signature)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
This bug was filed from the Socorro interface and is 
report bp-0e883053-f5e7-4121-9075-a08152150210.
=============================================================

working on steps to reproduce and seems a low volume crash for mac so far.
(Assignee)

Comment 1

3 years ago
Let's call this Cocoa widgets, since that's the code it happens in.

A quick search seems to show that this isn't new -- that it's been happening at low volume for quite a while.  That is if all crashes with this signature are actually the same crash:

https://crash-stats.mozilla.com/signature/?signature=-%5BNativeMenuItemTarget+menuItemHit%3A%5D
Component: General → Widget: Cocoa
The volume for this signature is relatively high in v39 and v40, it's three times
higher in 40.0a2 than in 38.0.1 for example.

Several user comments mentions clicking "Firefox > About Firefox".
(Assignee)

Comment 3

2 years ago
These crashes are strongly associated with an extension called µBlock or uBlock.  There are at least two distros, and multiple versions (perhaps multiple versions of each):

https://www.ublock.org/
https://addons.mozilla.org/en-US/firefox/addon/ublock/

https://github.com/chrisaljoudi/ublock
https://github.com/gorhill/uBlock
(Assignee)

Comment 4

2 years ago
uBlock is sometimes identified using the following uuid:
{2b10c1c8-a11f-4bad-fe9c-1c11e82cac42}
(Assignee)

Comment 5

2 years ago
I've been playing with various versions of uBlock, and still haven't found one with which I can reproduce this bug's crashes.  With all of them I've been using their default settings ... which may be the problem.
(Assignee)

Comment 6

2 years ago
Hi Deathamns.

I'm needinfoing you on this bug because you're a major contributor to uBlock and already have a bugzilla.mozilla.org account.  As I mentioned above, these crashes have a very strong correlation with uBlock -- in 100% of the crash reports I looked at, some version of uBlock is installed.

No, I don't think uBlock is to blame here.  In fact I think that'd be impossible.  But uBlock's presence does seem to make these crashes more likely.  And, in order to fix this bug, we need to find out how to reproduce it.  I suspect someone who knows uBlock well has a much better chance of coming up with good (nearly 100% effective) steps-to-reproduce than I do.

As mentioned above, several comments (in the crash logs) mention clicking on Firefox : About.  One mentions doing that to check for updates.

* clicked on firefox-about
* Crash happened as I was opening the 'About Firefox' window to check for Firefox updates.
* Every, every single time I open Firefox Aurora and I click "About Firefox" the application crashes if there is a new update.

Other comments are visible here:
https://crash-stats.mozilla.com/report/list?signature=-[NativeMenuItemTarget%20menuItemHit%3A]#tab-comments

Thanks in advance for whatever help you can give us!
Flags: needinfo?(deathamns)

Comment 7

2 years ago
(In reply to Steven Michaud [:smichaud] from comment #6)

I'm not sure how could I help here. The issue (for me) doesn't even seem to be related to the extension.
Many users have this add-on installed, so it may be just a coincidence.
I saw a few reports where it was not present, for example:
https://crash-stats.mozilla.com/report/index/ec6ceebd-dd9e-46e1-99b1-b886f2150618#extensions
https://crash-stats.mozilla.com/report/index/7e84c2f0-47b3-4b5a-b974-87c4c2150618#extensions
https://crash-stats.mozilla.com/report/index/2ee5027a-b899-4e75-b167-50cf02150618#extensions
https://crash-stats.mozilla.com/report/index/a3b5d8e9-76f5-4946-a810-417ed2150618#extensions
Flags: needinfo?(deathamns)
(Assignee)

Comment 8

2 years ago
> Many users have this add-on installed, so it may be just a coincidence.

Yes, you show that's possible.  Thanks for the info.

I'm going to put this bug aside for the time being.
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1193703
Looking at the assembly code for these crashes, it's likely that, once again, we're dealing with menuGroupOwner objects that have been deleted.  We've been plagued by this issue for years (as my old comment shows), without being able to find a good solution.  But just now I've thought of a potentially elegant workaround.  So I'll be working on this again for the next few days.

https://hg.mozilla.org/mozilla-central/annotate/ba43a48d3c52/widget/cocoa/nsMenuBarX.mm#l927
These are Mac topcrashers on the 43, 42 and 41 branches.
Keywords: topcrash-mac
(Assignee)

Updated

2 years ago
Assignee: nobody → smichaud
Created attachment 8653172 [details] [diff] [review]
Fix

This seems like a good way to work around the problem described in my old comment.  I don't know why I didn't think of it years ago :-)
Attachment #8653172 - Flags: review?(spohl.mozilla.bugs)
Comment on attachment 8653172 [details] [diff] [review]
Fix

Review of attachment 8653172 [details] [diff] [review]:
-----------------------------------------------------------------

\o/
Attachment #8653172 - Flags: review?(spohl.mozilla.bugs) → review+

Comment 14

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/55bea2798c68
Backed out for OSX crashes.
https://treeherder.mozilla.org/logviewer.html#?job_id=13387028&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/10a4c5fbcca7
I see the same crash manually (on quitting FF).  Sigh :-(

So this needs some more work ...
The problem is that I misinterpreted the Cocoa "create rule" (https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmRules.html).  I assumed I'd "own" any object created using either of the following:

-[[NSMutableSet alloc] initWithCapacity:]
+[NSMutableSet setWithCapacity:]

But that's wrong -- only methods 'whose name begins with “alloc”, “new”, “copy”, or “mutableCopy”' follow the "create rule".  So if I use the second method I need to also call "retain".

Yes, in some part of my mind I already knew this ... but it's been a while since I've had to deal with it explicitly :-(

New patch coming up.
Created attachment 8653712 [details] [diff] [review]
Fix rev1: What I will land

This should do the trick.  Carrying forward Stephen's r+.
Attachment #8653172 - Attachment is obsolete: true
Attachment #8653712 - Flags: review+
Comment on attachment 8653712 [details] [diff] [review]
Fix rev1: What I will land

By the way, I double-checked (by adding an NSLog() statement to -[MenuItemInfo dealloc]) that this won't result in us leaking all MenuItemInfo objects -- it won't.

Comment 20

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/10592387dea4
https://hg.mozilla.org/mozilla-central/rev/10592387dea4
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
These crashes are gone in m-c nightly builds made since my patch landed.  I think that counts as verification.
Status: RESOLVED → VERIFIED
Comment on attachment 8653712 [details] [diff] [review]
Fix rev1: What I will land

Approval Request Comment
[Feature/regressing bug #]: Very old bug
[User impact if declined]: Mac topcrasher will continue unfixed
[Describe test coverage new/current, TreeHerder]: Baked for five days on m-c -- crashes disappeared, no new problems reported
[Risks and why]: Low risk, simple patch
[String/UUID change made/needed]: None
Attachment #8653712 - Flags: approval-mozilla-beta?
Attachment #8653712 - Flags: approval-mozilla-aurora?
Comment on attachment 8653712 [details] [diff] [review]
Fix rev1: What I will land

Let's uplift to Aurora first and then to Beta in a few days once we know for sure that the crash has gone away without any unexpected fall outs. Aurora42+
Attachment #8653712 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

2 years ago
status-firefox41: --- → affected
status-firefox42: --- → affected
https://hg.mozilla.org/releases/mozilla-aurora/rev/fecd7b5233da
status-firefox42: affected → fixed
Comment on attachment 8653712 [details] [diff] [review]
Fix rev1: What I will land

I looked at crash stats again and noticed that this does not occur on 42.0a2 since 09-03 when the fix was landed in m-a. I will take that as a good sign that this fix has helped. Let's uplift to Beta41.
Attachment #8653712 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/c8b342e74901
status-firefox41: affected → fixed
You need to log in before you can comment on or make changes to this bug.