Closed Bug 471101 Opened 16 years ago Closed 16 years ago

Topcrash mostly on startup/shutdown of Firefox [@ nsMenuItemIconX::OnStopRequest(imgIRequest*, int)]

Categories

(Core :: Widget: Cocoa, defect, P1)

Product:
Core
Component:
Widget: Cocoa
Platform:
All
macOS
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9.2a1

People

(Reporter: jruderman, Assigned: smichaud)

References

Details

(Keywords: crash, topcrash, verified1.9.1)

Crash Data

Attachments

(2 files)

Patch for debugging
1.21 KB, patch
Details | Diff | Splinter Review
Fix (null check to avoid crashes)
845 bytes, patch
jaas
: review+
roc
: superreview+
Details | Diff | Splinter Review 
[@ nsMenuItemIconX::OnStopRequest] is currently the #1 topcrash on trunk.

I think I'm hitting this on more than 1 in 1000 startups.
Flags: blocking1.9.2?
Oops -- #1 topcrash on Mac, #3 topcrash overall.
I had the same crash some minutes ago while starting Minefield.

For reference the first 10 frames:

0  	XUL  	nsMenuItemIconX::OnStopRequest  	 widget/src/cocoa/nsMenuItemIconX.mm:481
1 	XUL 	imgRequestProxy::OnStopRequest 	modules/libpr0n/src/imgRequestProxy.cpp:549
2 	XUL 	imgRequest::OnStopRequest 	modules/libpr0n/src/imgRequest.cpp:771
3 	XUL 	nsJARChannel::OnStopRequest 	modules/libjar/nsJARChannel.cpp:841
4 	XUL 	nsInputStreamPump::OnStateStop 	netwerk/base/src/nsInputStreamPump.cpp:576
5 	XUL 	nsInputStreamPump::OnInputStreamReady 	netwerk/base/src/nsInputStreamPump.cpp:401
6 	XUL 	nsInputStreamReadyEvent::Run 	xpcom/io/nsStreamUtils.cpp:111
7 	XUL 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
8 	XUL 	NS_ProcessPendingEvents_P 	nsThreadUtils.cpp:180
9 	XUL 	nsBaseAppShell::NativeEventCallback 	widget/src/xpwidgets/nsBaseAppShell.cpp:121
10 	XUL 	nsAppShell::ProcessGeckoEvents 	widget/src/cocoa/nsAppShell.mm:374

As the comments indicate it mostly happens on startup/shutdown of Minefield. Adjusting the summary to better reflect the topic.
Summary: Topcrash [@ nsMenuItemIconX::OnStopRequest] → Topcrash mostly on startup/shutdown of Firefox [@ nsMenuItemIconX::OnStopRequest(imgIRequest*, int)]
FYI, A very similar crash stack is one of the main crashers within nsObjCExceptionLogAbort in 3.0.5 (the different bits in the middle may depend on the type of icon?); there aren't many comments, but the uptimes by and large are very short or fairly long. 

http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.0.5&platform=mac&query_search=stack&query_type=contains&query=nsMenuItemIconX%3A%3AOnStopFrame&date=&range_value=1&range_unit=days&do_query=1&signature=nsObjCExceptionLogAbort(NSException*)

There are 155 crashes just in the past day and 1044 in the last week; when I was looking at 3.0.5's nsObjCExceptionLogAbort reports with Sam earlier this week, this, bug 470864, bug 458961, and crashes in various printer mfgs' stuff accounted for the vast majority of the nsObjCExceptionLogAbort reports I looked at.
I hit this bug this morning after updating to the latest nightly. The odd thing was I didn't crash right away - in my case all of my tabs were frozen but I could operate in the menus. After clicking around a few times in the menu area I finally crashed and got this breakpad report: http://crash-stats.mozilla.com/report/index/0debb0b9-9153-4c24-9b30-c1ff02090107

This is the first time I have hit this crash.
The problem is that it happens really rarely for me. There was no way to find some good STR yet. Marcia, if you will see it often in future please try to remember what happens during restart, update of extensions, or both together. Do you have installed any new extensions?
No extensions in this profile except Google Desktop Search. I had a bunch of tabs open but I normally do when I restart.  I will try to see if I can reproduce the issue.
(In reply to comment #2 and comment #5)

Here's where these crashes happen:

NS_IMETHODIMP
nsMenuItemIconX::OnStopRequest(imgIRequest* aRequest,
                              PRBool       aIsLastPart)
{
->mIconRequest->Cancel(NS_BINDING_ABORTED);<-
  mIconRequest = nsnull;
  return NS_OK;
}

I suspect these crashes are due to mIconRequest having already been
deleted.

I'll dig further into this, to see if I can come up with some kind of
patch.
Assignee: joshmoz → smichaud
Priority: -- → P1
> I suspect these crashes are due to mIconRequest having already been
> deleted.

Actually no.  Marcia's crash is a null-pointer dereference.  So (if
her crashlog is any guide) these crashes happen because mIconRequest
is already null when nsMenuItemIconX::OnStopRequest() is called.

I *think* the only way this can happen is if
nsMenuItemIconX::OnStopRequest() has already been called at least once
on the same object.  If so, presumably the real bug is elsewhere,
probably somewhere in Gecko code.

If Marcia's crashlog is representative, the quickest and easiest fix
is to null-check mIconRequest before dereferencing it.  My debugging
patch does this.  But I've also added code to log when
nsMenuItemIconX::OnStopRequest() appears to have been called more than
once, or when it's called on the wrong object.

I've started a tryserver build made with this patch.  But the Mac
tryserver is queued three deep, so it will probably be many hours
before the build finishes.

When it does finish, I'll ask Marcia (and anyone else who can
reproduce these crashes) to try it out, and report any errors they see
in the console log.
Steven, thanks for this debugging patch. But the problem is that no-one of us can get Firefox crash reliable with that stack. Mostly it happens after an upgrade. And this feature isn't activated for tryserver builds. At least we could check if extension updates during start-up can cause it too.
(In reply to comment #11)

I'll hope that someone (probably you or Marcia) does figure out how to
reproduce this bug at least somewhat reliably.

If not, though, I think it'd be worthwhile to land the non-debugging
part of my patch anyway -- it's trivial, and can't do any harm.

It's true that we won't be able to get at the underlying bug (the true
cause of these crashes) unless someone finds a reliable STR for them.
But since this is a topcrasher, it makes sense to use my bandaid patch
(the null-check on mIconRequest) as a fallback, if we can't find
reliable STR.
For what it's worth, here's the tryserver build made with my debugging patch:

https://build.mozilla.org/tryserver-builds/2009-01-08_13:01-smichaud@pobox.com-bugzilla471101-debug/smichaud@pobox.com-bugzilla471101-debug-firefox-try-mac.dmg

Marcia and Henrik (and anyone else who's willing/able):  If you find halfway reliable STR for this bug, please try my tryserver build, and note what it logs to the system console.
The debugging patch output:
nsMenuItemIconX::OnStopRequest() called more than once on same request!! (aRequest 0x15A254B0)
(In reply to comment #14)

Thanks for the info.  Do you have reasonably reliable
steps-to-reproduce?  (Ones that work at least 25% of the time.)  If
so, what are they?
(In reply to comment #15)
> Thanks for the info.  Do you have reasonably reliable
> steps-to-reproduce?

Not reliable.
1. Quit Firefox.
2. Do other work for long time.
3. Start Firefox.

Sometimes crashes, maybe <10%.
I get this crash, even on the latest nightly build (released Jan 9th) reproducibly.  It crashes my browser several times a day.

1) Launch Shiretoko
2) I already have a bunch of tabs open, but it doesn't seem to matter which
3) Open a group of tabs from the bookmarks menu.  If it helps, the first one in my list is dilbert.com.
4) Watch the browser crash.

Here's a bunch of crash reports for it:
c0eadf42-334e-4daf-8a75-9aa332090109    1/9/09	10:13 AM
06f71dcc-562b-42aa-991b-bf9e02090109	1/9/09	1:27 AM
8cc4c3ed-3bdd-4952-bbdd-408d72090108	1/8/09	10:53 AM
706cd557-d254-4dff-83c3-249cb2090108	1/8/09	12:14 AM
edfd9f5e-8751-4378-8d8a-6ac252090107	1/7/09	9:10 PM
28a04ade-f4df-41a7-a638-e621a2090107	1/7/09	12:29 AM
I just crashed here too, definitely a null deref.
(In reply to comment #14)
After that, I  saw
nsMenuItemIconX::OnStopRequest() called more than once on same request!! (aRequest 0x18D00640)
and
nsMenuItemIconX::OnStopRequest() called more than once on same request!! (aRequest 0x19F28950)

But I've never seen "called on wrong request."
Sounds reasonable. Smichaud, I wasn't able to reproduce it within the last days. I hope the useful input from Smaug will help us here.
Hardware: x86 → All
Stephen, I thought I had fixed nsMenuItemIconX, but it's possible I didn't. Basically, imgIRequest::Cancel() happens asynchronously now, meaning that you can't call it from your destructor. I think that I changed nsMenuItemIconX to call imgIRequest::CancelAndForgetObserver correctly in bug 393936, but I admit I don't know all the interactions that are possible.

I'm not entirely clear on how this could cause an OnStopRequest mismatch, though.
(In reply to comment #23)

Thanks for the info.

> I'm not entirely clear on how this could cause an OnStopRequest
> mismatch, though.

We probably can't know the upstream cause of these crashes
until/unless we can reproduce them.

In the meantime, though, it's easy enough to fix the crashes
themselves (by doing a null-check).  On Monday I'll post a patch that
does this (just this, without the debugging cruft), do a tryserver
build, and start the review process.
Here are three more of the same crash:

965b0b2f-46a8-4632-a0e7-c0fab2090112	1/12/09	12:40 AM
8380e50c-cacb-45cf-9bf7-23b282090111	1/11/09	1:08 AM
44f3d6d2-f1cb-4875-b042-7b0d12090110	1/10/09	1:17 AM

All of them occur when I go to the bookmarks menu and try to open a set of bookmarks simultaneously.  Here are the urls that I'm opening, in the order in which the tabs are opened:

http://www.dilbert.com/fast
http://www.doonesbury.com/strip/dailydose/index.html
http://www.fborfw.com/strip_fix/
http://www.phdcomics.com/comics.php
http://dir.salon.com/topics/tom_tomorrow/index.html
http://www.userfriendly.org/static/
http://www.xkcd.com/
http://questionablecontent.net/index.php
http://www.kukuburi.com/current
http://picturesforsadchildren.com/

Note that other tabs are normally open when I do this, and often, there are multiple browser windows open, although this doesn't seem to be required.
(In reply to comment #25)
> All of them occur when I go to the bookmarks menu and try to open a set of
> bookmarks simultaneously.  Here are the urls that I'm opening, in the order in
> which the tabs are opened:

Michael, can you please give detailed steps? How do you open them simultaneously? Is it reproducible each time you do those steps?
This patch should fix the crashes.

To make it easier to decipher the underlying problem (once we're able
to reproduce it reliably), it also adds an assertion.

Since this is a topcrash, it should probably block 1.9.1.

Here's a tryserver build:

https://build.mozilla.org/tryserver-builds/2009-01-12_08:42-smichaud@pobox.com-bugzilla471101/smichaud@pobox.com-bugzilla471101-firefox-try-mac.dmg
Attachment #356562 - Flags: review?(joshmoz)
My patch from comment #27 is trivial, and should fix this bug's
crashes.  So there's basically no risk, and a very high reward.

My patch should probably get into FF3.1 beta3.
Flags: blocking1.9.1?
Attachment #356562 - Flags: review?(joshmoz) → review+
Comment on attachment 356562 [details] [diff] [review]
Fix (null check to avoid crashes)

Would be really nice to find the underlying cause but this seems like an appropriate safety measure.
Attachment #356562 - Flags: superreview?(roc)
In reply to comment 26, I have one or more windows open, normally with a number of tabs open in each one.  I typically select a tab that I don't need any more and do the following:

Some time after midnight (because the content I'm looking for updates by midnight), I go to:

Bookmarks -> Fun and Games -> Cartoons and Comics -> Open all in tabs.

The pages listed in comment 25 open.

About a second or two later, the browser crashes.  I submit a bug report.  The browser restarts.  The browser may or may not have saved those new tabs as being open.  If not, I do an open all in tabs again, and it typically does not crash.  But the next day, it does.  Note that the browser is restarted again at some point after this happens, because the latest Shiretoko nightly build gets downloaded.

This crash has happened every night.  My current crash reports show one to two crashes per day as shown below.

Also, not that my browser will typically get a spinning pinwheel for 30 seconds or so a short time after the restart.  This may be a separate bug.

e8ad29d8-85d6-4c29-8ee7-2afa22090113	1/13/09	1:34 AM
965b0b2f-46a8-4632-a0e7-c0fab2090112	1/12/09	12:40 AM
8380e50c-cacb-45cf-9bf7-23b282090111	1/11/09	1:08 AM
44f3d6d2-f1cb-4875-b042-7b0d12090110	1/10/09	1:17 AM
c0eadf42-334e-4daf-8a75-9aa332090109	1/9/09	10:13 AM
06f71dcc-562b-42aa-991b-bf9e02090109	1/9/09	1:27 AM
8cc4c3ed-3bdd-4952-bbdd-408d72090108	1/8/09	10:53 AM
706cd557-d254-4dff-83c3-249cb2090108	1/8/09	12:14 AM
edfd9f5e-8751-4378-8d8a-6ac252090107	1/7/09	9:10 PM
28a04ade-f4df-41a7-a638-e621a2090107	1/7/09	12:29 AM
a6a369ad-6967-4fb6-a35f-f0f402090106	1/6/09	11:25 PM
Michael, many thanks! Does it only happen with these special folder or are other bookmark folders also affected by "Open all in tabs"? If that only happens for that special kind it would be nice to get a list of those bookmarks or you could make a copy of this folder and delete bookmarks step by step to find the ones which cause the crash. It looks like you are the only person right now, who can reproduce this crash. Means more information would be really helpful.
Attachment #356562 - Flags: superreview?(roc) → superreview+
Comment on attachment 356562 [details] [diff] [review]
Fix (null check to avoid crashes)

As I say in comment #28, this patch, though trivial, should fix this
bug's crashes.  So it's zero risk and high reward.  And this bug is a
topcrasher.
Attachment #356562 - Flags: approval1.9.1?
Flags: blocking1.9.1? → blocking1.9.1+
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Keywords: fixed1.9.1
Attachment #356562 - Flags: approval1.9.1?
Flags: blocking1.9.2?
Target Milestone: --- → mozilla1.9.2a1
I filed bug 473841 on the assertion / figuring out why this happens.
Now that this has landed, perhaps we can hear back from Michael since he was the person that was reliably able to reproduce the crash.
No crash reports anymore on trunk since 20090113020320. Even none on 1.9.1 branch since 20090114020333.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
Ok.  I'd like to try this out, but I'm having a bit of trouble in doing so.  I was getting so many crashes that I reverted to beta 2 (probably about the same time my crash reports disappeared.  ;)  I've tried to find the beta 3 pre download link since then, but I'm just finding beta 2 and 3.2alpha and mozilla downloads for Windows (which doesn't help since I'm on OS X.)  Where is it, exactly?  :-)

Thanks,
Michael
Ok, I just tried the version lined above in comment 34, and then I tried Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090118 Minefield/3.2a1pre.  I've tried opening the same set of tabs on both, and not gotten a crash on either.  Yay!  I'll post again if the crash reappears later.

Of note, I also don't seem to be getting the 30 second spinning pinwheel when the browser starts either, and the browser also seems substantially faster.  It is, however, using much more memory than the nightlies that were crashing a few days ago.
Thanks for the feedback Michael!
Crash Signature: [@ nsMenuItemIconX::OnStopRequest(imgIRequest*, int)]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: