432257 - Crash FF3pre in [@ nsNavHistoryResult]

Status: RESOLVED INCOMPLETE

(Firefox :: Bookmarks & History, defect)

Description

[John J. Barton]

Hit this on 5/2, cleared my profile, worked no crashes. Returned today.
Happens with firebug 1.2a25X but only when I also use another, IBM plugin. 
Nothing in that plug uses nav history.  The crash happens after firebug completes its initial loading, so I think it is in those components that get loaded late. 

http://crash-stats.mozilla.com/report/index/9a689707-1ac3-11dd-83b5-001cc45a2ce4


0  	xpsp2res.dll  	xpsp2res.dll@0x202113  	
1 	js3250.dll 	js_Invoke 	mozilla/js/src/jsinterp.c:1312
2 	xul.dll 	nsXPCWrappedJSClass::CallMethod 	mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1523
3 	xul.dll 	nsXPCWrappedJS::CallMethod 	mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp:559
4 	xul.dll 	PrepareAndDispatch 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:114
5 	xul.dll 	SharedStub 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:141
6 	xul.dll 	nsNavHistoryFolderResultNode::OpenContainer 	mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp:3000

Comment 1

[John J. Barton]

If I turn on script tracing FBS_CREATION, the crash does not happen. This suggests a timing problem as tracing slows down the main path of FF startup.

Comment 2

[John J. Barton]

I can't get it to crash under windbg.

Comment 3

[John J. Barton]

By running with exactly the same command line options I did get the crash under windbg. 

First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=20202020 ebx=01b4f2e4 ecx=0448ff8f edx=0031c310 esi=0012e138 edi=02c8ba8c
eip=20202020 esp=0012e0ec ebp=0012e254 iopl=0         nv up ei pl zr na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010246
*** ERROR: Module load completed but symbols could not be loaded for C:\WINDOWS\system32\xpsp2res.dll
xpsp2res+0x202020:
20202020 0000            add     byte ptr [eax],al          ds:0023:20202020=00

The stack starts:
WARNING: Stack unwind information not available. Following frames may be wrong.
(ok, thanks a lot)

I can't figure out how to copy the stack from windbg more than one line, but here are two more frame below the ones above:
xul!nsCOMPtr<nsICSSStyleRule>::~nsCOMPtr<nsICSSStyleRule>(void)+0xc (FPO: [0,0,0]) (CONV: thiscall)
xul!nsNavHistoryContainerResultNode::SetContainerOpen(int aContainerOpen = 0)+0x1a (FPO: [2,0,0]) (CONV: stdcall) [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\toolkit\components\places\src\nsnavhistoryresult.cpp @ 454]

Comment 4

[timeless]

right click the stack window and select copy stack frames.

alternatively, use kp or some variant of k (.hh k) in the command window.

Comment 5

[John J. Barton]

(In reply to comment #4)
> right click the stack window and select copy stack frames.

The windbg window "Calls" does not respond to right click.

> 
> alternatively, use kp or some variant of k (.hh k) in the command window.
> 

ChildEBP RetAddr  
WARNING: Stack unwind information not available. Following frames may be wrong.
0012e254 6010b37e xpsp2res+0x202020
0012e310 60541630 js3250!js_Invoke(struct JSContext * cx = 0x00336180, unsigned int argc = 1, long * vp = 0x01b531b4, unsigned int flags = 0)+0x37e [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\js\src\jsinterp.c @ 1313]
0012e534 6052d838 xul!nsXPCWrappedJSClass::CallMethod(class nsXPCWrappedJS * wrapper = 0x042f8140, unsigned short methodIndex = 8, struct XPTMethodDescriptor * info = 0x0123e968, struct nsXPTCMiniVariant * nativeParams = 0x0012e570)+0x5c0 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\js\src\xpconnect\src\xpcwrappedjsclass.cpp @ 1524]
0012e54c 6065842e xul!nsXPCWrappedJS::CallMethod(unsigned short methodIndex = 0x8c00, struct XPTMethodDescriptor * info = 0x00000008, struct nsXPTCMiniVariant * params = 0x0012e628)+0x38 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\js\src\xpconnect\src\xpcwrappedjs.cpp @ 560]
0012e600 60658495 xul!PrepareAndDispatch(class nsXPTCStubBase * self = 0x042d8c00, unsigned int methodIndex = 8, unsigned int * args = 0x0012e628, unsigned int * stackBytesToPop = 0x0012e618)+0xe7 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp @ 114]
0012e61c 60495fa5 xul!SharedStub(void)+0x16 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp @ 142]
0012e630 606bba1f xul!nsNavHistoryFolderResultNode::OpenContainer(void)+0x5e [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\toolkit\components\places\src\nsnavhistoryresult.cpp @ 3000]
0012e640 605f7333 xul!nsCOMPtr<nsICSSStyleRule>::~nsCOMPtr<nsICSSStyleRule>(void)+0xc
0012e644 60658343 xul!nsNavHistoryContainerResultNode::SetContainerOpen(int aContainerOpen = 0)+0x1a [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\toolkit\components\places\src\nsnavhistoryresult.cpp @ 454]
0012e658 605457ab xul!NS_InvokeByIndex_P(class nsISupports * that = 0x00000000, unsigned int methodIndex = 0x3e50830, unsigned int paramCount = 0x12e6ec, struct nsXPTCVariant * params = 0x003dc070)+0x27 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp @ 102]
0012e6ec 606c1933 xul!XPCWrappedNative::CallMethod(class XPCCallContext * ccx = 0x04499f81, XPCWrappedNative::CallMode mode = CALL_METHOD (0))+0x4cb [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp @ 2371]
00000000 00000000 xul!XPCNativeSet::FindMember(long name = <Memory access error>, class XPCNativeMember ** pMember = <Memory access error>, class XPCNativeInterface ** pInterface = <Memory access error>, class XPCNativeSet * protoSet = 0x02e9938c, int * pIsLocal = <Memory access error>)+0x19 [e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\js\src\xpconnect\src\xpcinlines.h @ 480]

Comment 6

[timeless]

the window panel titles do. or you can [left]click the icon in the titles.

Comment 7

[Dietrich Ayala (:dietrich)]

John, is the IBM plugin publicly available?

Comment 8

[John J. Barton]

(In reply to comment #7)
> John, is the IBM plugin publicly available?
> 
No, sorry. Plus the problem seems sensitive to details. That is why I was trying windbg, to see if I could understand the problem enough to create a test case.

Comment 9

[John J. Barton]

Ok I was able to use shaver's hints about the Only True CVS.exe to see the source and I can stop before the access violation. It looks like nsNavHistoryFolderResultNode::OpenContainer is calling into javascript through a bunch of glue code.

00 js3250!js_Invoke(struct JSContext * cx = 0x0451a340, unsigned int argc = 0x4572bc0, long * vp = 0x00000008, unsigned int flags = 0x123e168)
01 xul!nsXPCWrappedJSClass::CallMethod(class nsXPCWrappedJS * wrapper = 0x04572bc0, unsigned short methodIndex = 8, struct XPTMethodDescriptor * info = 0x0123e168, struct nsXPTCMiniVariant * nativeParams = 0x0012e570)+0x5c0
02 xul!nsXPCWrappedJS::CallMethod(unsigned short methodIndex = 0x19a0, struct XPTMethodDescriptor * info = 0x00000008, struct nsXPTCMiniVariant * params = 0x0012e628)+0x38
03 xul!PrepareAndDispatch(class nsXPTCStubBase * self = 0x045719a0, unsigned int methodIndex = 8, unsigned int * args = 0x0012e628, unsigned int * stackBytesToPop = 0x0012e618)+0xe7
04 xul!SharedStub(void)+0x16
05 xul!nsNavHistoryFolderResultNode::OpenContainer(void)+0x5e
06 xul!nsCOMPtr<nsIDOMElement>::~nsCOMPtr<nsIDOMElement>(void)+0xc
07 xul!nsNavHistoryContainerResultNode::SetContainerOpen(int aContainerOpen = 0)+0x1a
08 xul!NS_InvokeByIndex_P(class nsISupports * that = 0x00000000, unsigned int methodIndex = 0x3dd4bd0, unsigned int paramCount = 0x12e6ec, struct nsXPTCVariant * params = 0x003341a0)+0x27
09 xul!XPCWrappedNative::CallMethod(class XPCCallContext * ccx = 0x00000001, XPCWrappedNative::CallMode mode = CALL_METHOD (0))+0x4cb
0a xul!XPCNativeSet::FindMember(long name = <Memory access error>, class XPCNativeMember ** pMember = <Memory access error>, class XPCNativeInterface ** pInterface = <Memory access error>, class XPCNativeSet * protoSet = 0x0031c180, int * pIsLocal = <Memory access error>)+0x19

0x03858211 "chrome://browser/content/places/toolbar.xml" line 0x243

I'm not 100% sure that the crash follows this because if I single step through the code it does not crash. If I re-run to my breakpoint in OpenContainer then go, I hit the crash.

Comment 10

[John J. Barton]

"chrome://browser/content/places/toolbar.xml" line 0x243

        containerOpened: function TV_V_containerOpened(aNode) {
          this.invalidateContainer(aNode);
        },

 

Comment 11

[John J. Barton]

Actually it looks like the "this" pointer used in OpenContainer is garbage. The Locals view says the nsNavHistoryContainerResultNode is at location FFFFFFFF.

Comment 12

[John J. Barton]

Still assuming I am on the stack that will crash, these are the lines of OpenContainer  
...
nsNavHistoryResult* result = GetResult();
  NS_ENSURE_TRUE(result, NS_ERROR_FAILURE);
  if (result->GetView())
    result->GetView()->ContainerOpened(
        static_cast<nsNavHistoryContainerResultNode*>(this));
  return NS_OK;
...
One thing that would make me pause is that this call to GetView() occurs during the call to SetView(). Here is more of the stack:
0 xul!nsNavHistoryFolderResultNode::OpenContainer+0x32
01 xul!nsNavHistoryContainerResultNode::SetContainerOpen+0x1a
02 xul!NS_InvokeByIndex_P+0x27
03 xul!XPCWrappedNative::CallMethod+0x4cb
04 xul!XPCNativeSet::FindMember+0x19
05 xul!XPCCallContext::XPCCallContext+0x245
06 xul!XPC_WN_OnlyIWrite_PropertyStub+0x3c
07 xul!XPC_WN_GetterSetter+0x231
08 js3250!js_Invoke+0x2bb
09 js3250!js_InternalInvoke+0xe7
0a js3250!js_NativeSet+0x11e
0b js3250!js_SetPropertyHelper+0x3ae
0c js3250!js_Interpret+0x18c9
0d js3250!js_Invoke+0x37e
0e js3250!js_InternalInvoke+0xe7
0f js3250!js_NativeSet+0x11e
10 js3250!js_SetPropertyHelper+0x3ae
11 js3250!js_SetProperty+0x1c
12 js3250!JS_SetProperty+0x4a
13 xul!nsXPCWrappedJSClass::CallMethod+0xafa
14 xul!nsXPCWrappedJS::CallMethod+0x38
15 xul!PrepareAndDispatch+0xe7
16 xul!SharedStub+0x16
17 xul!nsNavHistoryResult::SetViewer+0x28 
...

So SetViewer is calling javascript to set a viewer property and perhaps as a side effect the OpenContainer is being called which gets the viewer? 

Comment 13

[John J. Barton]

/browser/content/places/toolbar.xml line 404ish
       set result(val) {
          // some methods (e.g. getURLsFromContainer) temporarily null out the
          // viewer when they do temporary changes to the view, this does _not_
          // call setResult(null), but then, we're called again with the result
          // object which is already set for this viewer. At that point,
          // we should do nothing.
          if (this._self._result != val) {
            this._self._containerNodesMap = [];
            this._self._result = val;
            if (val) // this calls _rebuild through invalidateContainer
              val.root.containerOpen = true;
          }
          return val;
        },

  

Comment 14

[John J. Barton]

Ok, I commented out the if() in set result(val) and I do not crash. Does not mean those lines are bad, but it does mean I can get on with my work.

Comment 15

[John J. Barton]

One more observation: I don't crash unless I give a URL on the command line. I wonder if there is a test case on places for URLs from the command line?

Comment 16

[timeless]

try checking this from frame 01 or frame 03 (for frame 03 you'll have to read the code and figure out which variable it is).

Comment 17

[John J. Barton]

Here is the javascript stack in the method call that crashes. If I print the properties of the argument val and this._self, I do not crash.  (maybe timing or the getters I call in printing the properties?)

Places toolbar set result

JS frame :: chrome://browser/content/places/toolbar.xml :: anonymous :: line 411 
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0 
JS frame :: chrome://browser/content/places/toolbar.xml :: set_place :: line 289 
JS frame :: chrome://browser/content/browser.js :: initBookmarksToolbar :: line 126 
JS frame :: chrome://browser/content/browser.js :: delayedStartup :: line 775 

Comment 18

[John J. Barton]

Here are the properties of val.root, where val is the argument to the method that crashes.  Note that hasChildren is true, but childCount is invalid.

Printing this also prevents the crash.

Places toolbar set result val.root: sees object with typeof: 'object'; object contains:
[nsINavHistoryQueryResultNode.RESULT_TYPE_URI]=0;
[nsINavHistoryQueryResultNode.RESULT_TYPE_VISIT]=1;
[nsINavHistoryQueryResultNode.RESULT_TYPE_FULL_VISIT]=2;
[nsINavHistoryQueryResultNode.RESULT_TYPE_DYNAMIC_CONTAINER]=4;
[nsINavHistoryQueryResultNode.RESULT_TYPE_QUERY]=5;
[nsINavHistoryQueryResultNode.RESULT_TYPE_FOLDER]=6;
[nsINavHistoryQueryResultNode.RESULT_TYPE_SEPARATOR]=7;
[nsINavHistoryQueryResultNode.RESULT_TYPE_FOLDER_SHORTCUT]=9;
[nsINavHistoryResultNode.RESULT_TYPE_URI]=0;
[nsINavHistoryResultNode.RESULT_TYPE_VISIT]=1;
[nsINavHistoryResultNode.RESULT_TYPE_FULL_VISIT]=2;
[nsINavHistoryResultNode.RESULT_TYPE_DYNAMIC_CONTAINER]=4;
[nsINavHistoryResultNode.RESULT_TYPE_QUERY]=5;
[nsINavHistoryResultNode.RESULT_TYPE_FOLDER]=6;
[nsINavHistoryResultNode.RESULT_TYPE_SEPARATOR]=7;
[nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT]=9;
[nsINavHistoryContainerResultNode.RESULT_TYPE_URI]=0;
[nsINavHistoryContainerResultNode.RESULT_TYPE_VISIT]=1;
[nsINavHistoryContainerResultNode.RESULT_TYPE_FULL_VISIT]=2;
[nsINavHistoryContainerResultNode.RESULT_TYPE_DYNAMIC_CONTAINER]=4;
[nsINavHistoryContainerResultNode.RESULT_TYPE_QUERY]=5;
[nsINavHistoryContainerResultNode.RESULT_TYPE_FOLDER]=6;
[nsINavHistoryContainerResultNode.RESULT_TYPE_SEPARATOR]=7;
[nsINavHistoryContainerResultNode.RESULT_TYPE_FOLDER_SHORTCUT]=9;
[parent]=null;
[parentResult]=[xpconnect wrapped nsINavHistoryResult];
[uri]=place:folder=TOOLBAR;
[type]=6;
[title]=Bookmarks Toolbar;
[accessCount]=0;
[time]=0;
[icon]=null;
[indentLevel]=-1;
[viewIndex]=-1;
[propertyBag]=[xpconnect wrapped nsIWritablePropertyBag];
[bookmarkIndex]=-1;
[itemId]=3;
[dateAdded]=1209754472414809;
[lastModified]=1209754472508541;
[tags]=;
[containerOpen]=false;
[hasChildren]=true;
dumpProperties failed:[Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsINavHistoryContainerResultNode.childCount]"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: chrome://firebug/content/trace.js :: anonymous :: line 116"  data: no]
[getChild]=function getChild() {
    [native code]
};
[childrenReadOnly]=false;
[dynamicContainerType]=;
[appendURINode]=function appendURINode() {
    [native code]
};
[appendFolderNode]=function appendFolderNode() {
    [native code]
};
[RESULT_TYPE_URI]=0;
[RESULT_TYPE_VISIT]=1;
[RESULT_TYPE_FULL_VISIT]=2;
[RESULT_TYPE_DYNAMIC_CONTAINER]=4;
[RESULT_TYPE_QUERY]=5;
[RESULT_TYPE_FOLDER]=6;
[RESULT_TYPE_SEPARATOR]=7;
[RESULT_TYPE_FOLDER_SHORTCUT]=9;

Comment 19

[John J. Barton]

This version of the crashing method does not crash.  It suggests there is a bug in the implementation of nsINavHistoryContainerResultNode.

        set result(val) {
          // some methods (e.g. getURLsFromContainer) temporarily null out the
          // viewer when they do temporary changes to the view, this does _not_
          // call setResult(null), but then, we're called again with the result
          // object which is already set for this viewer. At that point,
          // we should do nothing.
	  if (val.root) {
	      var qi = var.root instanceof Components.interfaces.nsINavHistoryContainerResultNode;
	  }

          if (this._self._result != val) {
            this._self._containerNodesMap = [];
            this._self._result = val;
            if (val) // this calls _rebuild through invalidateContainer
              val.root.containerOpen = true;
          }
          return val;
        },

Comment 20

[John J. Barton]

How to patch FF3 to fix this crash on my local copy:
Minefield\chrome\browser.jar
unzip into folder browser 
Edit Minefield\chrome\browser\content\browser\places\toolbar.xml line 409
in folder Minefield\chrome\browser\content zip 'browser' folder, creates content.zip. Rename to browser.jar. Copy to .. overwriting browser.jar.
New jar file will be 1/2 size because it is compressed but original was not.

Comment 21

[John J. Barton]

(In reply to comment #19)
> This version of the crashing method does not crash.  
Bah. I just added a syntax error that prevented the crash.

Comment 22

[Marco Bonardo [:mak]]

i suspect this was due to cycle collecting, we fixed a couple issues in this area where an half dead result was still receiving updates. Can you reproduce in current trunk or 1.9.2 branch?

Comment 23

[Jesse Ruderman on Windows]

John, I'm marking this bug as incomplete due to the lack of response to comment 22.

Comment 24

[John J. Barton]

Yes thanks jesse, that is fine.

Comment 25

[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