419891 - Firefox crashes in [@ nsStandardURL::SetFileExtension]

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008022704 Minefield/3.0b4pre ID:2008022704

I started Firefox after an update with the latest session and after closing some tabs Firefox crashes. I'm not able to reproduce this issue at the moment.

Breakpad id: bp-164481bd-e560-11dc-9d1c-001a4bd46e84

0  	nsStandardURL::SetFileExtension(nsACString_internal const&)  	 mozilla/netwerk/base/src/nsStandardURL.cpp:2353
1 	nsNavHistoryResult::OnItemVisited(long long, long long, long long) 	mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp:4088
2 	nsNavBookmarks::OnVisit(nsIURI*, long long, long long, long long, long long, unsigned int, unsigned int*) 	mozilla/toolkit/components/places/src/nsNavBookmarks.cpp:2625
3 	nsNavHistory::AddVisit(nsIURI*, long long, nsIURI*, int, int, long long, long long*) 	mozilla/toolkit/components/places/src/nsNavHistory.cpp:2546
4 	nsNavHistory::AddVisitChain(nsIURI*, long long, int, int, nsIURI*, long long*, long long*, long long*) 	mozilla/toolkit/components/places/src/nsNavHistory.cpp:4145

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/base/src/nsStandardURL.cpp&rev=1.109&mark=2353#2353

Seems that input is null?

Comment 1

[Shawn Wilsher :sdwilsh]

hrm - why is that stack frame not complete?

Comment 2

[Jesse Ruderman]

Attachment: Jesse's stack trace

I hit a similar-looking crash [@ nsNavBookmarks::OnVisit] while I was opening a bunch of tabs (including Gmail).  I was using a Mac trunk debug build from around 1am.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Is this being caused by Places or Session Restore?

Comment 4

[Honza Bambas (:mayhemer)]

Attachment: Possible fix

My theory is that 'void *ref' in nsMaybeWeakPtr_base::GetValueAs is not initialized to NULL. 

When a null pointer is added to the array of observers (nsMaybeWeakPtrArray<nsINavBookmarkObserver> mObservers) then ENUMERATE_WEAKARRAY -> nsMaybeWeakPtrArray::ElementAt -> nsMaybeWeakPtr::GetValue returns some random mess because nsMaybeWeakPtr::mPtr is then NULL.

This patch fixes that concrete issue that should be fixed anyway even if it is not cause of this bug.

I cannot reproduce this bug on my Mac/Win. Is there anyone who can?

Comment 5

[Honza Bambas (:mayhemer)]

And one more question: are the observers, mainly added as weak ref, accessed/referenced on several threads?

Comment 6

[Shawn Wilsher :sdwilsh]

The observers in places should only be accessed on the main thread.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

(In reply to comment #4)
> I cannot reproduce this bug on my Mac/Win. Is there anyone who can?

Sorry, it was a one-time crash. Haven't seen it again until now. So no clean STR are available. 

Comment 8

[Reed Loden [:reed]]

Checking in toolkit/components/places/src/nsMaybeWeakPtr.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsMaybeWeakPtr.cpp,v  <--  nsMaybeWeakPtr.cpp
new revision: 1.3; previous revision: 1.2
done

Comment 9

[Tracy Walker [:tracy]]

seeing one crash report with this signature on 3/17
http://crash-stats.mozilla.com/report/index/3dbe7168-f65d-11dc-ad24-001a4bd46e84

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

(In reply to comment #9)
> seeing one crash report with this signature on 3/17
> http://crash-stats.mozilla.com/report/index/3dbe7168-f65d-11dc-ad24-001a4bd46e84

The build id is a bit old: 2008030317

Due to I don't have a valid testcase I cannot verify it. Let's wait for a while if someone runs into this crash.

Comment 11

[Tracy Walker [:tracy]]

yes, but the fix went in prior to 3/17 (on 3/12)  Waiting for a longer bake is no problem before actually marking it verified is no problem.

Comment 12

[Honza Bambas (:mayhemer)]

(In reply to comment #11)
> yes, but the fix went in prior to 3/17 (on 3/12)  Waiting for a longer bake is
> no problem before actually marking it verified is no problem.
> 

Just to clarify: build ID you reported is 2008030317, this is 2008-03-03-17 (where 17 is probably the build order number during 3.3.2008 date). Also, the patch is not present in beta 4 as seen from CVS graph and target milestone.

Comment 14

[Tracy Walker [:tracy]]

ah, sorry about that.

Comment 15

[Tracy Walker [:tracy]]

verified per no new crash reports

Comment 16

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv