320489 - Crash [@ nsDocShell::RestoreFromHistory]

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Brian Ryner (not reading)]

There are several crashes in Firefox 1.5 at nsDocShell::RestoreFromHistory.  It's not clear what users are doing to trigger it, other than hitting Back.

Typical crash report:

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=12876245

Note that the stack is partially trashed.  There are some other reports where the stack is not trashed, such as:

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=12853525

These reports seem to generally not have the correct line number at the top of the stack.  It seems like something might be out of whack with the session history tree.

Comment 1

[Adam Guthrie]

Incident ID: 12853525
Stack Signature	nsDocShell::RestoreFromHistory() c24693c7
Product ID	Firefox15
Build ID	2005111116
Trigger Time	2005-12-11 15:21:13.0
Platform	LinuxIntel
Operating System	Linux 2.6.12-12mdk
Module	firefox-bin + (004c65ce)
URL visited	
User Comments	
Since Last Crash	0 sec
Total Uptime	0 sec
Trigger Reason	SIGSEGV: Segmentation Fault: (signal 11)
Source File, Line No.	/builds/tinderbox/Fx-Mozilla1.8/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/docshell/base/nsDocShell.cpp, line 848
Stack Trace 	
nsDocShell::RestoreFromHistory()  [/builds/tinderbox/Fx-Mozilla1.8/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/docshell/base/nsDocShell.cpp, line 848]
HandleRestorePresentationEvent()  [/builds/tinderbox/Fx-Mozilla1.8/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/docshell/base/nsDocShell.cpp, line 1040]
PL_HandleEvent()  [/builds/tinderbox/Fx-Mozilla1.8/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/xpcom/threads/plevent.c, line 689]
PL_ProcessPendingEvents()  [/builds/tinderbox/Fx-Mozilla1.8/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/xpcom/threads/plevent.c, line 623]
nsEventQueueImpl::ProcessPendingEvents()  [/builds/tinderbox/Fx-Mozilla1.8/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/xpcom/threads/nsEventQueue.cpp, line 421]
event_processor_callback()  [/builds/tinderbox/Fx-Mozilla1.8/Linux_2.4.21-27.0.4.ELsmp_Depend/mozilla/widget/src/gtk2/nsAppShell.cpp, line 67]

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This shows up in Camino, too; it's the #5 topcrash of 1.0b1 at the moment:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsDocShell::RestoreFromHistory&vendor=MozillaOrg&product=Camino10&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

Comment 3

[Brian Ryner (not reading)]

The most likely correct stack for this comes from the win32 reports, which place the crash at line 5470, not line 848.  If that's correct, then mLSHE is probably null when we try to get the child shell list.  And, it must have gotten nulled out sometime after we accessed it earlier in this method.  I'm a little stumped as to how this could happen.  If anyone has any steps to reproduce this crash it would help quite a bit.

It's also possible that this stack is misleading -- 2 of the 3 platforms insist the crash is at line 848, which doesn't even match up with the reported function.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

gcc seems to generate wrong debug information for inline functions (that, or talkback is interpreting them wrongly). i.e., for an inline function, it shows the line number where the inline function is defined, not where it's called.

Comment 5

[Brian Ryner (not reading)]

Ok, so line 848 might actually refer to nsCOMPtr.h, which would correspond to calling through mLSHE with operator->().

Comment 6

[Brian Ryner (not reading)]

I haven't been able to come up with a definite way to cause a crash here.  We know mLSHE is good up to where we call DestroyChildren().  I guess it's possible that doing that might null out mLSHE somehow via an unload handler (which would be bad in other respects, since end up with a null mOSHE).  I don't see anything else that would be likely to affect mLSHE.

Comment 7

[Brian Ryner (not reading)]

I think this can be fixed on the branch by landing an adaptation of bug 312117's patch.  This constrains usage of mLSHE to before we call DestroyChildren.

Comment 8

[Brian Ryner (not reading)]

Attachment: like this

This is pretty straightforward, it just calls the getters sooner.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 207473 [details] [diff] [review]
like this

>Index: nsDocShell.cpp
>+    nsCOMPtr<nsISupportsArray> refreshURIList;
>+    mLSHE->GetRefreshURIList(getter_AddRefs(refreshURIList));

The |mLSHE->SetRefreshURIList(nsnull);| line got lost, I think.  Put that back in, and r=bzbarsky

Comment 10

[Brian Ryner (not reading)]

Comment on attachment 207473 [details] [diff] [review]
like this

Requesting branch approval.  This is an adaptation of code that's already on the trunk.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 207473 [details] [diff] [review]
like this

a=dveditz for 1.8/1.8.0.1 branches. Please add the fixed1.8.1 and fixed1.8.0.1 keywords when checked in.

Comment 12

[Brian Ryner (not reading)]

checked in on branches

Comment 13

[Marcia Knous [:marcia]]

Comment 6 indicates we don't have a definitive steps to reproduce this crash, so difficult for QA to verify this one.