Closed Bug 1131473 Opened 8 years ago Closed 7 years ago

crash in -[NativeMenuItemTarget menuItemHit:]


(Core :: Widget: Cocoa, defect)

Not set



Tracking Status
firefox41 --- fixed
firefox42 --- fixed
firefox43 --- fixed


(Reporter: cbook, Assigned: smichaud)



(Keywords: crash, topcrash-mac)

Crash Data


(1 file, 1 obsolete file)

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.
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:
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".
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):
uBlock is sometimes identified using the following uuid:
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.
Hi Deathamns.

I'm needinfoing you on this bug because you're a major contributor to uBlock and already have a 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:[NativeMenuItemTarget%20menuItemHit%3A]#tab-comments

Thanks in advance for whatever help you can give us!
Flags: needinfo?(deathamns)
(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:
Flags: needinfo?(deathamns)
> 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.
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.
These are Mac topcrashers on the 43, 42 and 41 branches.
Keywords: topcrash-mac
Assignee: nobody → smichaud
Attached patch Fix (obsolete) — Splinter Review
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]

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

Attachment #8653172 - Flags: review?(spohl.mozilla.bugs) → review+
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" (  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.
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.
Closed: 7 years ago
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.
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+
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+
You need to log in before you can comment on or make changes to this bug.