open apprunner to bugzilla query page look for all bugs submitted by buster, no other fields filled in on the results page, sort by bug # hit back button, let page redisplay hit back button again, get the crash below because aState is garbage: FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x0284d0b0, nsILayoutHistoryState * 0x02bd3790) line 1102 + 21 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x0284d790, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x0283a090, nsILayoutHistoryState * 0x02bd3790) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x02838f10, nsILayoutHistoryState * 0x02bd3790) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x0283a570, nsILayoutHistoryState * 0x02bd3790) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x02839ee0, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x0283b7f0, nsILayoutHistoryState * 0x02bd3790) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x0283b900, nsILayoutHistoryState * 0x02bd3790) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x0283e120, nsILayoutHistoryState * 0x02bd3790) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x0283d080, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x0283d140, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x027cd1a0, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x02b5df60, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x02b2fad0, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x02b2d030, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x02b2d170, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x02bd3030, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x02a26800, nsILayoutHistoryState * 0x02bd3790) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x02ae4cc0, nsIFrame * 0x02aefa80, nsILayoutHistoryState * 0x02bd3790) line 1120 + 20 bytes PresShell::ContentAppended(PresShell * const 0x02ae10d8, nsIDocument * 0x02a264f0, nsIContent * 0x02ae5d3c, int 8) line 1735 nsDocument::ContentAppended(nsDocument * const 0x02a264f0, nsIContent * 0x02ae5d3c, int 8) line 1574 nsHTMLDocument::ContentAppended(nsHTMLDocument * const 0x02a264f0, nsIContent * 0x02ae5d3c, int 8) line 1044 HTMLContentSink::NotifyBody() line 276 HTMLContentSink::CloseForm(HTMLContentSink * const 0x02a249e0, const nsIParserNode & {...}) line 2113 CNavDTD::CloseForm(const nsIParserNode & {...}) line 2478 + 31 bytes CNavDTD::CloseContainer(const nsIParserNode & {...}, nsHTMLTag eHTMLTag_form, int 0) line 2719 + 12 bytes CNavDTD::HandleEndToken(CToken * 0x01cf13e0) line 1500 + 24 bytes NavDispatchTokenHandler(CToken * 0x01cf13e0, nsIDTD * 0x02ae4960) line 257 + 12 bytes CTokenHandler::operator()(CToken * 0x01cf13e0, nsIDTD * 0x02ae4960) line 80 + 14 bytes CNavDTD::HandleToken(CNavDTD * const 0x02ae4960, CToken * 0x01cf13e0, nsIParser * 0x02a24b50) line 754 + 18 bytes CNavDTD::BuildModel(CNavDTD * const 0x02ae4960, nsIParser * 0x02a24b50, nsITokenizer * 0x02ae6fb0, nsITokenObserver * 0x00000000, nsIContentSink * 0x02a249e0) line 564 + 20 bytes nsParser::BuildModel() line 1001 + 34 bytes nsParser::ResumeParse(nsIDTD * 0x00000000, int 0) line 922 + 11 bytes nsParser::OnDataAvailable(nsParser * const 0x02a24b54, nsIChannel * 0x02a58140, nsISupports * 0x00000000, nsIInputStream * 0x02a228e8, unsigned int 0, unsigned int 2399) line 1337 + 19 bytes nsDocumentBindInfo::OnDataAvailable(nsDocumentBindInfo * const 0x02a582d0, nsIChannel * 0x02a58140, nsISupports * 0x00000000, nsIInputStream * 0x02a228e8, unsigned int 0, unsigned int 2399) line 1371 + 32 bytes nsChannelListener::OnDataAvailable(nsChannelListener * const 0x02a58210, nsIChannel * 0x02a58140, nsISupports * 0x00000000, nsIInputStream * 0x02a228e8, unsigned int 0, unsigned int 2399) line 1613 nsHTTPResponseListener::OnDataAvailable(nsHTTPResponseListener * const 0x02a23550, nsIChannel * 0x02a5aaa0, nsISupports * 0x02a58140, nsIInputStream * 0x02a228e8, unsigned int 28672, unsigned int 2399) line 186 + 47 bytes nsOnDataAvailableEvent::HandleEvent(nsOnDataAvailableEvent * const 0x02828910) line 359 nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x028288c0) line 152 + 12 bytes PL_HandleEvent(PLEvent * 0x028288c0) line 541 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x00d1a850) line 500 + 9 bytes _md_EventReceiverProc(HWND__ * 0x00180272, unsigned int 49308, unsigned int 0, long 13740112) line 970 + 9 bytes

just got the crash again. this time the URL sequence was: www.search.com, enter "nissan xterra" click on first hit: http://www.go.com/Center/Automotive/Spec/Nissan_Xterra click on http://www.go.com/Center/Automotive/Spec/Research_tools_and_resources_for_buying _a_car/Makes_and_models/Nissan/Nissan_Xterra?style1=SE+4X2+Year+2000& make1=& cid1=& cs1=& cid2=& cs2=& m1=& m2=& vnow (link name = SE 4x2 2000) click on the image to go to http://www.go.com/Center/Automotive/Spec/Research_tools_and_resources_for_buying _a_car/Makes_and_models/Nissan/Nissan_Xterra?style1=SE+4X2+Year+2000& apg=large_pic& cs1=SE+4X2+Year+2000& cid1=28215& m1=Nissan& make1=Nissan& svx=autos_toolbox_large_pi hit the back button reproduced twice. stack: FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a64f10, nsILayoutHistoryState * 0x02b7d150) line 1102 + 21 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a5b0d0, nsILayoutHistoryState * 0x02b7d150) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a5ba80, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a5bb20, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a5eb80, nsILayoutHistoryState * 0x02b7d150) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a59890, nsILayoutHistoryState * 0x02b7d150) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a5a1e0, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a4da40, nsILayoutHistoryState * 0x02b7d150) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a4e0b0, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a4e950, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a4e7c0, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a39d00, nsILayoutHistoryState * 0x02b7d150) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a447c0, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a44900, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a40730, nsILayoutHistoryState * 0x02b7d150) line 1127 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a46990, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a47aa0, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a463b0, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a47040, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02a44040, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes FrameManager::RestoreFrameState(FrameManager * const 0x028f9840, nsIFrame * 0x02918030, nsILayoutHistoryState * 0x02b7d150) line 1120 + 20 bytes PresShell::ContentAppended(PresShell * const 0x028f8658, nsIDocument * 0x0301ab30, nsIContent * 0x02915f3c, int 0) line 1735 nsDocument::ContentAppended(nsDocument * const 0x0301ab30, nsIContent * 0x02915f3c, int 0) line 1574 nsHTMLDocument::ContentAppended(nsHTMLDocument * const 0x0301ab30, nsIContent * 0x02915f3c, int 0) line 1044 HTMLContentSink::NotifyBody() line 276 HTMLContentSink::WillInterrupt(HTMLContentSink * const 0x0301bf10) line 1770 CNavDTD::WillInterruptParse(CNavDTD * const 0x028fa4a0) line 3111 + 27 bytes nsParser::ResumeParse(nsIDTD * 0x00000000, int 0) line 958 nsParser::OnDataAvailable(nsParser * const 0x0301a184, nsIChannel * 0x023e4790, nsISupports * 0x00000000, nsIInputStream * 0x03018c38, unsigned int 0, unsigned int 4208) line 1337 + 19 bytes nsDocumentBindInfo::OnDataAvailable(nsDocumentBindInfo * const 0x023e3760, nsIChannel * 0x023e4790, nsISupports * 0x00000000, nsIInputStream * 0x03018c38, unsigned int 0, unsigned int 4208) line 1371 + 32 bytes nsChannelListener::OnDataAvailable(nsChannelListener * const 0x028ca8e0, nsIChannel * 0x023e4790, nsISupports * 0x00000000, nsIInputStream * 0x03018c38, unsigned int 0, unsigned int 4208) line 1613 nsHTTPResponseListener::OnDataAvailable(nsHTTPResponseListener * const 0x03018cb0, nsIChannel * 0x030144c0, nsISupports * 0x023e4790, nsIInputStream * 0x03018c38, unsigned int 28672, unsigned int 4208) line 186 + 47 bytes nsOnDataAvailableEvent::HandleEvent(nsOnDataAvailableEvent * const 0x02a1e560) line 359 nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x02a18fd0) line 152 + 12 bytes PL_HandleEvent(PLEvent * 0x02a18fd0) line 541 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x00d1a850) line 500 + 9 bytes _md_EventReceiverProc(HWND__ * 0x041308e4, unsigned int 49308, unsigned int 0, long 13740112) line 970 + 9 bytes

I see this also when clicking the Forward button (as well as the Back button) using the 1999092708 build on NT.

I see this on linux also, marking platform all. To reproduce on linux simply: 1. Launch and watch http://www.mozilla.org come up as start page 2. Enter http://www.yahoo.com into location bar and hit enter 3. Watch yahoo load and hit the back arrow 4. Watch mozilla page load again and hit forward button You will then crash. Chofmann said to cc: troy on this one.

I have nothing to do with the restore frame code. Changing Cc to Nisheeth

just crashed with the 1999092608 build on MacOS 8.5, very similar stack trace. Changing platform to All for real this time ;-)

Reassigning to Radha.

putting on my m10 radar.

cc'ing Pollmann. I don't see webshell or session History anywhere in the stack. So, I'm not sure if the aState is null because webshell didn't pass the right state. I think eric will know better.

Radha, could it be that the state has been NS_RELEASED? I'll take a look to see what's going on.

I'd bet a dollar or twenty that Warren caused this bug with his most recent checking to webshell, which coincidentally, was to fix a history state leak on 09/26. Warren, Session history must maintain a reference to the session history until it is deleted from the list completely. The pages may be revisited multiple times.

s/must maintain a reference to the session history/must maintain a reference to the page state/g; :S

Radha, one way to fix this might be to make nsWebshell::mHistory be an nsISupportsArray, have nsHistoryEntry implement nsISupports, and write a proper destructor that will free the page state. (RemoveElementAt of nsISupportsArray calls NS_RELEASE on the element). In the meantime, you might want to do something quicker like back out Warren's fix, manually call NS_RELEASE before calling mHistory.RemoveElementAt(), and manually NS_RELEASE each element of the history array when you are done with it. I'll check this in my tree to see if it fixes the bug. Unfortunately, Purify doesn't like my 64Mb machine, so I can't as easily check to see if it causes massive leaks to reappear.

Backed out Warren's leak fix in my local tree and the crash went away. Here is the diff: Index: nsWebShell.cpp =================================================================== RCS file: /cvsroot/mozilla/webshell/src/nsWebShell.cpp,v retrieving revision 1.261 diff -r1.261 nsWebShell.cpp 2153c2153 < nsCOMPtr<nsISupports> historyState; --- > nsISupports* historyState; 2156c2156 < rv = GetHistoryState(getter_AddRefs(historyState)); --- > rv = GetHistoryState(&historyState); I'd strongly recommend talking to Warren before backing this out, as I believe this is exactly the type of leak that he, Vidur, and Rick Potts have been killing themselves over lately.

Well, my simple test must have not been sufficient to get at the behavior here. I was going to a new page, going back, then going forward to somewhere else (presumably pruning the history entry), but I saw it leaked. Also, I thought I saw these being leaked on shutdown. I'd argue that the way this code was written it wasn't clear that it didn't leak. Usually the caller doesn't addref for the called method (in this case SetHistoryObjectForIndex) -- that's confusing.

Eric, warren, I saw your comments. The history state is being released by session History in nsSessionHistory.cpp, when a page no-longer becomes reachable. So, I don't know why it is leaking there. I just now got my copy of purify. I'll look in to it. Radha

I have to agree with Warren here. In fact, I'd leave his change in, then instead, addref aState inside nsSessionHistory::SetHistoryState: Index: nsSessionHistory.cpp =================================================================== RCS file: /cvsroot/mozilla/xpfe/appshell/src/nsSessionHistory.cpp,v retrieving revision 1.20 diff -r1.20 nsSessionHistory.cpp 268a269,270 > NS_IF_RELEASE(mHistoryState); > NS_ADDREF(aState); (This also fixes the crash) You are already NS_IF_RELEASING mHistoryState on destruction, so I assume you had planned on maintaining a reference to it?

radha, have you tried the fix eric suggests? we'd like to fix this rather noticeable crashing behaviour before we branch m10

Hmmm, you're going to want to make that an NS_IF_ADDREF. In the spirit of dogfood, I was fixing another bug with my patched apprunner and noticed that sometimes aState will be null and cause a crash. Index: nsSessionHistory.cpp =================================================================== RCS file: /cvsroot/mozilla/xpfe/appshell/src/nsSessionHistory.cpp,v retrieving revision 1.20 diff -r1.20 nsSessionHistory.cpp 268a269,270 > NS_IF_RELEASE(mHistoryState); > NS_IF_ADDREF(aState);

*** Bug 15011 has been marked as a duplicate of this bug. ***

I still the crash with pollman's patches. The problem is, the addref in nsSessionHistory gets nullified when historyState goes out of scope in nsWebShell::LoadURL(). The following patch + warren's changes + eric's changes doesn't cause the crash. chiffon:radha [15] cvs diff nsWebShell.cpp Index: nsWebShell.cpp =================================================================== RCS file: /cvsroot/mozilla/webshell/src/nsWebShell.cpp,v retrieving revision 1.261 diff -r1.261 nsWebShell.cpp 2789a2790 > NS_ADDREF(*aLayoutHistoryState);

If you just need to get something in for M10, this sounds fine. I'll have to say though, 1) I don't see the crash and 2) after a little digging around, it's seems strange to me that you need this extra addref. Looking at nsWebShell::LoadURI() there is: 1) An addref in nsWebShell::GetHistoryState->nsPresShell::GetHistoryState->NS_NewLayoutHistoryState 2) An addref inside nsSessioinHistory::SetHistoryObjectForIndex->nsHistoryEntry::SetHistoryState 1) A release when the nsCOMPtr "historyState" goes out of scope. This should leave you with a refcount of one, and you should not loose the instance. Is there yet another extra release somewhere I'm missing?

I think Radha found the extra release. She also suggested adding an addref to GetHistoryState. We're working through the refcounting via email now...

Here's the leak I was trying to fix: [W] MLK: Memory leak of 16 bytes from 1 block allocated in NS_NewLayoutHistoryState(nsILayoutHistoryState * *) Distribution of leaked blocks Allocation location new(UINT) [new.cpp:23] NS_NewLayoutHistoryState(nsILayoutHistoryState * *) [nsLayoutHistoryState.cpp:77] PresShell::GetHistoryState(nsILayoutHistoryState * *) [nsPresShell.cpp:1644] nsWebShell::GetHistoryState(nsISupports * *) [nsWebShell.cpp:2789] nsWebShell::LoadURI(nsIURI *,char const*,nsIInputStream *,int,UINT,UINT,nsISupports *,WORD const*) [nsWebShell.cpp:2156] nsWebShell::LoadURL(WORD const*,char const*,nsIInputStream *,int,UINT,UINT,nsISupports *,WORD const*) [nsWebShell.cpp:2331] nsWebShell::LoadURL(WORD const*,nsIInputStream *,int,UINT,UINT,nsISupports *,WORD const*) [nsWebShell.cpp:1927] nsBrowserWindow::DispatchMenuItem(int) [nsBrowserWindow.cpp:573] nsNativeBrowserWindow::DispatchMenuItem(int) [nsWinMain.cpp:122] HandleBrowserEvent [nsBrowserWindow.cpp:340] nsWindow::DispatchEvent(nsGUIEvent *,nsEventStatus&) [nsWindow.cpp:338] nsWindow::DispatchWindowEvent(nsGUIEvent *) [nsWindow.cpp:358] nsWindow::ProcessMessage(UINT,UINT,long,long *) [nsWindow.cpp:2248] nsWindow::WindowProc(HWND__ *,UINT,UINT,long) [nsWindow.cpp:447] TranslateMessageEx [user32.dll] DispatchMessageA [USER32.dll] nsNativeViewerApp::Run(void) [nsWinMain.cpp:71] main [nsWinMain.cpp:135] mainCRTStartup [crtexe.c:338]

radha...is this fixed in today's build...as far as the crashing?

*** Bug 15072 has been marked as a duplicate of this bug. ***

*** Bug 15369 has been marked as a duplicate of this bug. ***

radha has checked into the m10 branch and I don't see the crash in the Friday night m10 builds.. marking fixed

verified