Closed Bug 433525 Opened 13 years ago Closed 13 years ago

crash [@ nsNavHistoryQueryResultNode::IsContainersQuery()]

Categories

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

defect

Tracking

()

VERIFIED FIXED

People

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

References

()

Details

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

Crash Data

Attachments

(1 file, 1 obsolete file)

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.
If anyone can reproduce this, it'd be very helpful to get the places.sqlite for that profile.
Assignee: nobody → dietrich
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
Attached 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)
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)
> 
> 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).
Attachment #320842 - Attachment is obsolete: true
Whiteboard: [has patch][needs review marco] → [has patch]
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?]
This is now topcrash #1 for Firefox 3 RC1.

http://crash-stats.mozilla.com/topcrasher/byversion/Firefox/3.0
Blocks: 434590
I was able to reproduce this crash with the steps in bug 434590, and able to confirm that this patch fixes it.
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
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.
Whiteboard: [has patch][has reviews][RC2?] → [has patch][has reviews][RC2?][RC2+]
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+
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)
Fixed?
http://hg.mozilla.org/cvs-trunk-mirror/index.cgi/rev/6b5944d1c06d
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews][RC2?][RC2+] → [RC2+][needs to land in mozilla-central]
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.