crash [@ nsNavHistoryQueryResultNode::IsContainersQuery()]

VERIFIED FIXED

Status

()

defect
P1
critical
VERIFIED FIXED
11 years ago
8 years ago

People

(Reporter: samuel.sidler+old, Assigned: dietrich)

Tracking

({crash, regression, topcrash})

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 -
wanted-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [RC2+][needs to land in mozilla-central], crash signature, )

Attachments

(1 attachment, 1 obsolete attachment)

Crash stats shows a new topcrash that started in the 2008050600 builds. This is topcrash #8 in the last week.

Sample stack from bp-f90a22cc-20c7-11dd-83ba-0013211cbf8a:

Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsNavHistoryQueryResultNode::IsContainersQuery 	mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp:2098
1 	xul.dll 	nsNavHistoryQueryResultNode::CanExpand 	mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp:2071
2 	xul.dll 	nsNavHistoryQueryResultNode::GetHasChildren 	mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp:2164
3 	xul.dll 	NS_InvokeByIndex_P 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
4 	xul.dll 	XPCWrappedNative::CallMethod 	mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2369

There aren't many comments yet, but uptime seems to imply that this is happening at startup for many people or within the first few minutes of usage.
Flags: wanted1.9.0.x?
Flags: blocking-firefox3?
Guys, any ideas what can call this? I tend to suspect the Most Visited smartfolder.
FYI, this has dropped off to topcrash 27, with the following frequency over the past two weeks: 

All     Win     OSX    Linux    Sol
48  	 42 	 6 	 0 	 0 

and seems to be dropping off in terms of frequency with later builds.
Err... beltzner, this is #8 in the past week with that frequency. It's lower in the last two weeks (#27) because it didn't occur until the 2008050600 builds.

It does seem like there's a drop off, but I think that's just nightly users switching to RC1 (like, QA). Looking at the Table in the URL above, you'll see the drop off. But I think we should wait until RC1 crash feedback before decidedly blocking on this.
Assignee

Comment 5

11 years ago
If anyone can reproduce this, it'd be very helpful to get the places.sqlite for that profile.
Assignee: nobody → dietrich
Assignee

Comment 6

11 years ago
Looks like it's a back-end bug exposed by the fix for bug 431817. Fix coming up.
Status: NEW → ASSIGNED
Depends on: 431817
just to confirm this crash, i was running during my RC1 FFT run now 2 times into this crash on XP SP3. 

The Crash happened out of the blue on different scenarios and also when i just surfing. 

http://crash-stats.mozilla.com/report/index/588d5b51-2147-11dd-bfbf-0013211cbf8a?p=1
Assignee

Comment 8

11 years ago
Posted patch potential fix (obsolete) — Splinter Review
looks like mOptions can be accessed before it's initialized, via GetHasChildren().

builds here:

https://build.mozilla.org/tryserver-builds/2008-05-13_17:44-dietrich@mozilla.com-bug433525-isContainersQueryCrash/
Attachment #320842 - Flags: review?(mak77)
Assignee

Updated

11 years ago
Whiteboard: [has patch][needs review marco]
Blocks: 431817
No longer depends on: 431817
Comment on attachment 320842 [details] [diff] [review]
potential fix

Index: toolkit/components/places/src/nsNavHistoryResult.cpp
===================================================================
+//  Safe options getter, ensures queries are parsed first.
+nsNavHistoryQueryOptions*
+nsNavHistoryQueryResultNode::Options()
+{

nit: this file comment style has an empty line between comment and function def

+  nsresult rv = VerifyQueriesParsed();
+  if (NS_FAILED(rv))
+    return nsnull;
+  NS_ASSERTION(mOptions, "Options invalid, cannot generate fro mURI");

nit: from URI

+
+function run_test() {
+  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].getService(Ci.nsINavHistoryService);
+  var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].getService(Ci.nsINavBookmarksService);

nit: cut at 80 and try catch

+  var options = histsvc.getNewQueryOptions();
+  options.maxResults = 1;
+  options.resultType = options.RESULTS_AS_URI
+  var query = histsvc.getNewQuery();

missing ; after RESULTS_AS_URI

+
+  // now check via the saved search path
+  var queryURI = histsvc.queriesToQueryString([query], 1, options);
+  bmsvc.insertBookmark(bmsvc.toolbarFolder, uri(queryURI), 0 /* first item */, "test query");

nit: 80 chars


the code is ok but the crash test does not crash for me without the patch, isn't it supposed to verify the crash?
Attachment #320842 - Flags: review?(mak77)
Assignee

Comment 10

11 years ago
> 
> the code is ok but the crash test does not crash for me without the patch,
> isn't it supposed to verify the crash?
> 

doesn't crash for me either. tomcat is going to try the build, since he can reproduce the crash (i tried on windows, couldn't reproduce).
Assignee

Comment 11

11 years ago
Attachment #320842 - Attachment is obsolete: true
Assignee

Updated

11 years ago
Whiteboard: [has patch][needs review marco] → [has patch]
Assignee

Updated

11 years ago
Priority: -- → P1
Would be good to get this fixed ASAP, not a showstopper on its own.
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
Whiteboard: [has patch] → [has patch][RC2?]

Comment 13

11 years ago
This is now topcrash #1 for Firefox 3 RC1.

http://crash-stats.mozilla.com/topcrasher/byversion/Firefox/3.0
Blocks: 434590
Assignee

Comment 14

11 years ago
I was able to reproduce this crash with the steps in bug 434590, and able to confirm that this patch fixes it.
Assignee

Updated

11 years ago
Attachment #320917 - Flags: review?(mano)
Comment on attachment 320917 [details] [diff] [review]
comments fixed (checked in)

r=mano
Attachment #320917 - Flags: review?(mano) → review+
Duplicate of this bug: 434590
Assignee

Updated

11 years ago
Whiteboard: [has patch][RC2?] → [has patch][has reviews][RC2?]
(In reply to comment #12)
> Would be good to get this fixed ASAP, not a showstopper on its own.
> 

Well, under windows, I am unable to get the RC1 build to run more than an hour or two without hitting this crash.
During testing of  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008051202 Firefox/3.0 I got in this state and was consistently crashing until I started a new session and then the crash went away. I was not doing anything in Places when I crashed. The profile I am running with contains data from Firefox 2.
Marcia, do you still have the affected sessionstore.js? Can you share it?
(In reply to comment #18)
> During testing of  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US;
> rv:1.9) Gecko/2008051202 Firefox/3.0 I got in this state and was consistently
> crashing until I started a new session and then the crash went away. I was not
> doing anything in Places when I crashed. The profile I am running with contains
> data from Firefox 2.
> 

What do you mean by you were not doing anything in Places when you crashed?  Almost everything you do in the browser involves a history update which uses Places.
I meant I was not doing anything like the STR in Bug 434590. First crash was clicking on the Dock icon.

Updated

11 years ago
Whiteboard: [has patch][has reviews][RC2?] → [has patch][has reviews][RC2?][RC2+]

Comment 22

11 years ago
Comment on attachment 320917 [details] [diff] [review]
comments fixed (checked in)

a+ schrep for 3.0.1 or RC2 please land on cvs trunk.
Attachment #320917 - Flags: approval1.9+
Assignee

Comment 23

11 years ago
Checking in toolkit/components/places/src/nsNavHistoryResult.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp,v  <--  nsNavHistoryResult.cpp
new revision: 1.147; previous revision: 1.146
done
Checking in toolkit/components/places/src/nsNavHistoryResult.h;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryResult.h,v  <--  nsNavHistoryResult.h
new revision: 1.57; previous revision: 1.56
done
RCS file: /cvsroot/mozilla/toolkit/components/places/tests/unit/test_433525_hasChildren_crash.js,v
done
Checking in toolkit/components/places/tests/unit/test_433525_hasChildren_crash.js;
/cvsroot/mozilla/toolkit/components/places/tests/unit/test_433525_hasChildren_crash.js,v  <--  test_433525_hasChildren_crash.js
initial revision: 1.1
done
Attachment #320917 - Attachment description: comments fixed → comments fixed (checked in)

Comment 24

11 years ago
Fixed?

Comment 25

11 years ago
http://hg.mozilla.org/cvs-trunk-mirror/index.cgi/rev/6b5944d1c06d
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews][RC2?][RC2+] → [RC2+][needs to land in mozilla-central]

Updated

11 years ago
Duplicate of this bug: 435059
There is no real way to verify this, is there, other than looking at the crash data?
Should be able to verify with the steps in bug 434590 (see comment 14). Possibly in bug 435059 too.

But yes, the full verification will be that it disappears from the topcrash report for RC2.
Flags: wanted1.9.0.x+
There are still crashes reported by users who are still running Firefox 3.0. These are 66 crashes within the last month. For later builds this stack isn't listed anymore. Now more than 4 month later I believe we can mark this bug as verified. At least I cannot reproduce it with the latest builds.
Status: RESOLVED → VERIFIED
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
Crash Signature: [@ nsNavHistoryQueryResultNode::IsContainersQuery()]
You need to log in before you can comment on or make changes to this bug.