418379 - crash [@ nsNavHistoryFolderResultNode::FindChildById(__int64, unsigned int*)]

Status: RESOLVED DUPLICATE of bug 419891

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

Description

[Samuel Sidler (old account; do not CC)]

Firefox 3 beta 3 has a new topcrash. This still occurs on the trunk and appears to be Windows-only.

See also: bp-3270140c-deda-11dc-9265-001a4bd43ed6

Crashing Thread
Frame 	Signature 	Source
0 	nsNavHistoryFolderResultNode::FindChildById(__int64, unsigned int*) 	mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp:3215
1 	nsNavHistoryResult::OnItemChanged(__int64, nsACString_internal const&, int, nsACString_internal const&) 	mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp:4034
2 	nsNavBookmarks::OnItemAnnotationSet(__int64, nsACString_internal const&) 	mozilla/toolkit/components/places/src/nsNavBookmarks.cpp:2726
3 	nsAnnotationService::RemoveItemAnnotations(__int64) 	mozilla/toolkit/components/places/src/nsAnnotationService.cpp:1573
4 	nsNavBookmarks::RemoveItem(__int64) 	mozilla/toolkit/components/places/src/nsNavBookmarks.cpp:999
5 	NS_InvokeByIndex_P 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
6 	XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) 	mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2339

(Side note: If this is crashing in toolkit code, why am I filing it in the Firefox product?)

Comment 1

[chris hofmann]

currently ranked #12 for beta3.

looks like people might be getting to this crash from a variety of ways manipulating bookmarks.  comments include:

Was adding a bookmark with CTRL+D and tried to right-click add a new folder to the save dialog.

Trying to rename the folder "Segnalibri Smart"
tried to rename smart bookmarks folder
Renaming a live bookmark. Hit enter and *poof* crashola
Mozilla has broken when I tried to change smart bookmarks name.

trying to import bookmarks

selected delete from bookmark
Occured while trying to delete "Smart Bookmarks" from the bookmarks toolbar.

I was trying to add a new bookmark...

then there are some comments with similar stack traces that are harder to explain..

...Fresh install of Firefox and Vista, crashed on first run

...the last couple of days it is behaving strange, starting the process and not opening the browser window. I did not install anything (extentions or plug-ins) in these couple of days. firefox 2 starts normaly

Comment 2

[Ondrej Brablc]

So the code is crashing here:

  typedef nsTArray<nsNavHistoryFolderResultNode*> FolderObserverList;
  ...
  FolderObserverList* list = BookmarkFolderObserversForId(folderId, PR_FALSE);
  ...
  nsNavHistoryFolderResultNode* folder = list->ElementAt(i);
  if (folder) {
    PRUint32 nodeIndex;
    nsNavHistoryResultNode* node = folder->FindChildById(aItemId, &nodeIndex);

The "if (folder)" is always true and thus gives false sense of protection. In the very likely case, that someone has released the node from memory without removing it from the list, the pointer will still be non zero, but any operation on it will lead to crash.

Comment 3

[chris hofmann]

for some reason this has dramatically dropped off in b4, and is no where near the top #100 anymore.   Anyone have thoughts on what might have reduced the problem?

Here is a link to the few reports that we have in this area for b4.

http://crash-stats.mozilla.com/?do_query=1&product=Firefox&version=Firefox%3A3.0b4&query_search=signature&query_type=contains&query=nsNavHistoryFolderResultNode&date=&range_value=2&range_unit=weeks

Comment 4

[Todd Agulnick]

fyi, I didn't think that Foxmarks users were seeing this, but I just received such a report:

http://crash-stats.mozilla.com/report/index/90f0585e-f4a9-11dc-89e8-001a4bd46e84

Comment 5

[Ondrej Brablc]

Attachment: Stacks showing how previously wrong code could have been reached

Most likely fixed with bug 419891 (checkin b5pre 20080312). I was able to reach the following line in debugger (without the patch it could lead to crash):
http://mxr.mozilla.org/mozilla/source/toolkit/components/places/src/nsMaybeWeakPtr.cpp#61 

No crash references in this bug are newer than the patch. And I believe it is legal, that some of the observers disappears before it unregister itself (the C++ code seems to be unregistering OK).

Comment 8

[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