Closed Bug 422743 Opened 16 years ago Closed 16 years ago

Crash in nsVoidArray::Count() during bookmark synchronization

Categories

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

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
Firefox 3

People

(Reporter: toddsf, Assigned: ondrej)

Details

Attachments

(1 file, 1 obsolete file)

Two users have reported identical crashes in beta 4:

http://crash-stats.mozilla.com/report/index/a4de5aba-f066-11dc-a63a-001a4bd43e5c
http://crash-stats.mozilla.com/report/index/f9851600-edc1-11dc-ab76-001a4bd46e84

These crashes have nsNavHistoryFolderResultNode::FindChildById(__int64, unsigned int*) in their call stack, and so may be related to https://bugzilla.mozilla.org/show_bug.cgi?id=418379. But the exact location of the crash is different, so it might be possible that they're not the same.
And also this one:

http://crash-stats.mozilla.com/report/index/72eda1bd-f0bd-11dc-8dee-001a4bd43ef6

... which has a slightly different (perhaps more informative?) stack frame.
Talked to Todd offline, and Foxmark users are getting this crash regularly. Requesting blocking.
Flags: blocking-firefox3?
Priority: -- → P1
Target Milestone: --- → Firefox 3
Flags: blocking-firefox3? → blocking-firefox3+
Assignee: nobody → ondrej
Dietrich/Ondrej: does this need to block beta 5? If so, do we have an ETA or idea towards a solution?
Target Milestone: Firefox 3 → Firefox 3 beta5
(In reply to comment #4)
> Dietrich/Ondrej: does this need to block beta 5? If so, do we have an ETA or
> idea towards a solution?
> 

Does not need to block B5, I think. It's windows only, and 89th topcrash for B4, 35th for B5pre, and there's no immediate leads other than that Foxmarks can trigger it. We'll work with Todd to get it reproduced and resolved asap.
Priority: P1 → P2
Target Milestone: Firefox 3 beta5 → Firefox 3
Status: NEW → ASSIGNED
(In reply to comment #3)
> Talked to Todd offline, and Foxmark users are getting this crash regularly.
> Requesting blocking.

It does not seem to be that easy to find Foxmarks for Fx3. I have signed up to be a beta tester (username brablc) and hope that Todd can approve me soon.
(In reply to comment #6)
 
> It does not seem to be that easy to find Foxmarks for Fx3. I have signed up to
> be a beta tester (username brablc) and hope that Todd can approve me soon.
> 

We'll get you added to the beta program in a couple of hours; apologies for the delay.
(In reply to comment #0)
> These crashes have nsNavHistoryFolderResultNode::FindChildById(__int64,
> unsigned int*) in their call stack, and so may be related to
> https://bugzilla.mozilla.org/show_bug.cgi?id=418379. But the exact location of
> the crash is different, so it might be possible that they're not the same.

Yes it seems like duplicate - we see different stacks with this kind of memory corruption (bug 419891 and bug 418379). The fix is dated b5pre 20080312 and none of the reports here is newer.

Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → DUPLICATE
I asked one of our users who was seeing this crash consistently to try with the latest nightly build. It's still crashing in nsVoidArray:Count(), although he says it does get further along in the sync process.

Breakpad report is here: http://crash-stats.mozilla.com/report/index/10e7a5d9-f789-11dc-a4c2-001a4bd43ef6
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
FWIW, I believe I'm the user in question. I'm getting the daily Minefield updates and testing with newly created profiles more or less daily. Here's the crash report from today's Minefield and latest Foxmarks build.

http://crash-stats.mozilla.com/report/index/1419e100-fad1-11dc-8af5-001a4bd43ed6
I have had this crash consistently since I started testing Foxmarks around March12 (in fact one of the reports Todd submitted was from me).  Tonight I got this crash most recently with the nightly for 3/25/2008.  You can find the latest crash at:

http://crash-stats.mozilla.com/report/index/e5665c66-faf9-11dc-9461-001a4bd43ed6
(In reply to comment #10)
> FWIW, I believe I'm the user in question. I'm getting the daily Minefield
> updates and testing with newly created profiles more or less daily. Here's the
> crash report from today's Minefield and latest Foxmarks build.
> 
> http://crash-stats.mozilla.com/report/index/1419e100-fad1-11dc-8af5-001a4bd43ed6
> 

(In reply to comment #11)
> I have had this crash consistently since I started testing Foxmarks around
> March12 (in fact one of the reports Todd submitted was from me).  Tonight I got
> this crash most recently with the nightly for 3/25/2008.  You can find the
> latest crash at:
> 
> http://crash-stats.mozilla.com/report/index/e5665c66-faf9-11dc-9461-001a4bd43ed6
> 

Matt and Leland, I cannot reproduce the bug, so I would appreciate your help very much. Can you try to create minimalistic steps to reproduce with new profile. I would like to see something like:
1) Create new profile.
2) Install FoxMarks extension.
3) Download bookmarks from FoxMarks -> crash.

If you have something like this, it would be nice, if you could play with the bookmark size - for instance deleting half of the bookmarks on FoxMarks and trying again. If it fails, than removing another half and so on. So that we find the smallest bookmark set that leads to a crash.

If it does not happen always, please try to find out the conditions when it happens. How long was Firefox running, what were you doing...
I've been doing the 1, 2, 3 (lather, rinse, repeat) each new build of Minefield for about a week. I delete my "default" profile, create a new one, then try to sync.

Today, I did the next step, which was to start deleting large folders of bookmarks and trying again. I started with around 2100 bookmarks. Next I tried about 1650, and that crashed. Then I dropped to 930 or so, and it worked. I'll try adding more in and see if I can get closer to the critical number.

When I had the full set, the crash was 100%. I never got a successful sync. I could skip syncing altogether, and Minefield would be fine, but any attempt to sync would crash.

http://crash-stats.mozilla.com/report/index/53331d11-fb93-11dc-a50f-001a4bd43e5c
http://crash-stats.mozilla.com/report/index/d29d8cd7-fb93-11dc-9313-001a4bd43e5c
http://crash-stats.mozilla.com/report/index/36ea3b7d-fb96-11dc-b794-001a4bd43e5c
http://crash-stats.mozilla.com/report/index/ca75ef00-fb96-11dc-b454-001a4bd43e5c
I was not able to reproduce any crash, but I was able to get in situation where the condition below is true and saw, that the folder node was still registered:

+nsNavHistoryFolderResultNode::~nsNavHistoryFolderResultNode()
+{
+  if (mIsRegisteredFolderObserver && mResult)
+   mResult->RemoveBookmarkFolderObserver(this, mItemId);
+}
 
STR:

1) Import some 200+ bookmarks split into 3 groups (root folders).
2) Open bookmarks sidebar and expand all folders.
3) Close/open the sidebar several times - this kicks garbage collection after a while (you can see this by having a breakpoint in nsNavHistoryResult::~nsNavHistoryResult()).
4) Now delete one of the root folders (you would prefer doing this when sidebar is closed).
5) And sync Foxmarks->Firefox using Advanced tab.

If there were not the removal of the observer, it could happen, that bookmarks being imported by Foxmarks will lead to a notification (as we can see with SetItemDateAdded) that will be delivered to a node that has just be destructed, without being removed from the folder observer. This could lead to a crash.
Attachment #312237 - Flags: review?(dietrich)
Status: REOPENED → ASSIGNED
Attachment #312237 - Attachment is obsolete: true
Attachment #312239 - Flags: review?(dietrich)
Attachment #312237 - Flags: review?(dietrich)
Comment on attachment 312239 [details] [diff] [review]
Unregister removed node to avoid crash (expanding tab to spaces)

r=me
Attachment #312239 - Flags: review?(dietrich) → review+
Foxmarks testers: thanks for the input and crashreport links. Please re-open this if you continue to see the crash.

Checking in toolkit/components/places/src/nsNavHistoryResult.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp,v  <--  nsNavHistoryResult.cpp
new revision: 1.138; previous revision: 1.137
done
Checking in toolkit/components/places/src/nsNavHistoryResult.h;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryResult.h,v  <--  nsNavHistoryResult.h
new revision: 1.55; previous revision: 1.54
done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
With my large bookmark set, I still saw the following, with two different sync attempts:
http://crash-stats.mozilla.com/report/index/3ca87c05-fe93-11dc-9ead-001a4bd43ef6
http://crash-stats.mozilla.com/report/index/7ab4fb82-fe93-11dc-a233-001a4bd43ef6

I spent a while over the weekend trying deleting groups of bookmarks, then creating fixed sized folders to keep adding to try and find some magic number. As near as I can tell, no particular number of bookmarks was triggering it, but there was definitely an issue somehow with the structure of the folders, or the depth, or the naming.
(In reply to comment #18)
> With my large bookmark set, I still saw the following, with two different sync
> attempts:
> http://crash-stats.mozilla.com/report/index/3ca87c05-fe93-11dc-9ead-001a4bd43ef6
> http://crash-stats.mozilla.com/report/index/7ab4fb82-fe93-11dc-a233-001a4bd43ef6
> 
> I spent a while over the weekend trying deleting groups of bookmarks, then
> creating fixed sized folders to keep adding to try and find some magic number.
> As near as I can tell, no particular number of bookmarks was triggering it, but
> there was definitely an issue somehow with the structure of the folders, or the
> depth, or the naming.

Thanks for reporting this, it looks like a separate issue - I have filed bug 426261 for this.
Ondrej,

Can you confirm that the following report describes a crash on a build that *didn't* yet include your patch?

http://crash-stats.mozilla.com/report/index/54362b1c-fcf8-11dc-b8c6-001a4bd43ef6

That is, did build 2008032805 include the fix or not?

None of the users I've been in contact with who were having this problem have been successful syncing with the most recent builds, although it seems like these users are now experiencing different problem(s). I see that you've opened another bug for Matt's new issue (thanks!); another user who was experiencing the crash in nsVoidArray::Count() says that Firefox simply shuts down without offering to submit a crash report when he tries to sync -- not sure what to do in this case.

If we can facilitate in some way, please let me know.

(In reply to comment #20)
> Ondrej,
> 
> Can you confirm that the following report describes a crash on a build that
> *didn't* yet include your patch?
> 
> http://crash-stats.mozilla.com/report/index/54362b1c-fcf8-11dc-b8c6-001a4bd43ef6
> 
> That is, did build 2008032805 include the fix or not?

This bugs was checked in at 2008-03-28 11:32. This means that it could not have been included in build 2008032805. As I understand it from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/ the times are in (PST/PDT) zone.

> another user who was experiencing the crash in nsVoidArray::Count() says 
> that Firefox simply shuts down without offering to submit a crash report 
> when he tries to sync -- not sure what to do in this case.

I suggest waiting until I come with a solution for the other crashing bug. If this will not solve the problem for this person, only then we should try to get more information.
Foxmarks folks, work on the crash is now occurring in bug 426261. Per the comments there, can you try and reproduce the crash with this build?

https://build.mozilla.org/tryserver-builds/2008-04-08_15:42-dietrich@mozilla.com-Bug426261-CrashWhileSync/

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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: