The caller of RestoreFromHistory() should use strong ref

RESOLVED FIXED

Status

()

Core
Document Navigation
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

9 years ago
Created attachment 383246 [details] [diff] [review]
patch

I got the crash when running chrome tests and I had some local changes.
I think my changes somehow made the bug visible, but as far as I see, 
we need to keep the docshell alive.
RestoreFromHistory dispatches some dom events, so anything can happen,
but RestorePresentationEvent has only raw ref.

#0  0x00000032d7097581 in nanosleep () from /lib64/libc.so.6
#1  0x00000032d70973a4 in sleep () from /lib64/libc.so.6
#2  0x00002aaaaaaf5245 in ah_crap_handler (signum=11)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/toolkit/xre/nsSigHandlers.cpp:149
#3  0x00002aaaaaaf5e08 in nsProfileLock::FatalSignalHandler (signo=11) at nsProfileLock.cpp:216
#4  <signal handler called>
#5  0x0000000000000000 in ?? ()
#6  0x00002aaab22cfdae in nsDocShell::GetPresShell (this=0x22698a0, aPresShell=0x7fff09a3a110)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/docshell/base/nsDocShell.cpp:1592
#7  0x00002aaab22ebbac in nsDocShell::RestoreFromHistory (this=0x22698a0)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/docshell/base/nsDocShell.cpp:6872
#8  0x00002aaab22ec548 in nsDocShell::RestorePresentationEvent::Run (this=<value optimized out>)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/docshell/base/nsDocShell.cpp:6376
#9  0x00002aaaab2c36aa in nsThread::ProcessNextEvent (this=0x66c780, mayWait=1, result=0x7fff09a3a39c)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/xpcom/threads/nsThread.cpp:516
#10 0x00002aaaab27e567 in NS_ProcessNextEvent_P (thread=0x22698a0, mayWait=1) at nsThreadUtils.cpp:230
#11 0x00002aaab9830392 in nsBaseAppShell::Run (this=0xec70c0)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:170
#12 0x00002aaab70c981b in nsAppStartup::Run (this=0xc41ac0)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:193
#13 0x00002aaaaaaeafae in XRE_main (argc=<value optimized out>, argv=<value optimized out>, aAppData=<value optimized out>)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/toolkit/xre/nsAppRunner.cpp:3340
#14 0x0000000000400eca in main (argc=5, argv=0x0)
    at /home/smaug/mozilla/mozilla_cvs/hg/mozilla/browser/app/nsBrowserApp.cpp:156
Attachment #383246 - Flags: superreview?(bzbarsky)
Attachment #383246 - Flags: review?(bzbarsky)
Comment on attachment 383246 [details] [diff] [review]
patch

This looks ok, but why not just make the member an nsRefPtr?
Attachment #383246 - Flags: superreview?(bzbarsky)
Attachment #383246 - Flags: superreview+
Attachment #383246 - Flags: review?(bzbarsky)
Attachment #383246 - Flags: review+
(Assignee)

Comment 2

9 years ago
because of ref cycles. (which in practice might be problem only during shutdown in this case)
What ref cycles?  Docshell isn't holding a strong ref to the event, is it?
(Assignee)

Comment 4

9 years ago
uh, right. I'll make mDocShell to be nsRefPtr
(Assignee)

Comment 5

9 years ago
Created attachment 383429 [details] [diff] [review]
v2
(Assignee)

Comment 6

9 years ago
http://hg.mozilla.org/mozilla-central/rev/c34dfea7629a
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Depends on: 498736
Group: core-security
You need to log in before you can comment on or make changes to this bug.