Closed Bug 320489 Opened 15 years ago Closed 15 years ago

Crash [@ nsDocShell::RestoreFromHistory]

Categories

(Core :: DOM: Navigation, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: bryner, Assigned: bryner)

Details

(4 keywords, Whiteboard: [need testcase])

Crash Data

Attachments

(1 file)

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.
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]
Severity: normal → critical
Summary: Crash @nsDocShell::RestoreFromHistory → Crash [@ nsDocShell::RestoreFromHistory]
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.
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.
Ok, so line 848 might actually refer to nsCOMPtr.h, which would correspond to calling through mLSHE with operator->().
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.
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.
Attached patch like thisSplinter Review
This is pretty straightforward, it just calls the getters sooner.
Attachment #207473 - Flags: review?(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
Attachment #207473 - Flags: review?(bzbarsky) → review+
Comment on attachment 207473 [details] [diff] [review]
like this

Requesting branch approval.  This is an adaptation of code that's already on the trunk.
Attachment #207473 - Flags: approval1.8.1?
Attachment #207473 - Flags: approval1.8.0.1?
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.
Attachment #207473 - Flags: approval1.8.1?
Attachment #207473 - Flags: approval1.8.1+
Attachment #207473 - Flags: approval1.8.0.1?
Attachment #207473 - Flags: approval1.8.0.1+
checked in on branches
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [need testcase]
Comment 6 indicates we don't have a definitive steps to reproduce this crash, so difficult for QA to verify this one.
Crash Signature: [@ nsDocShell::RestoreFromHistory]
You need to log in before you can comment on or make changes to this bug.