Closed Bug 303725 Opened 17 years ago Closed 17 years ago

null pointer dereference crash [@ PresShell::RetargetEventToParent]

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

x86
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: dbaron, Assigned: bryner)

References

Details

(Keywords: crash, topcrash+, verified1.8)

Crash Data

Attachments

(2 files)

Since yesterday (I think because of the checkin for bug 301804), I've been
crashing quite frequently with the assertion followed by null pointer dereference:

###!!! ASSERTION: No docshell for container.: 'docShell', file
/builds/trunk/mozilla/layout/base/nsPresShell.cpp, line 5942
Break: at file /builds/trunk/mozilla/layout/base/nsPresShell.cpp, line 5942

nsDebugImpl::Break(char const*, int)
(/builds/trunk/mozilla/xpcom/base/nsDebugImpl.cpp:326)
nsDebugImpl::Assertion(char const*, char const*, char const*, int)
(/builds/trunk/mozilla/xpcom/base/nsDebugImpl.cpp:261)
nsDebug::Assertion(char const*, char const*, char const*, int)
(/builds/trunk/obj/firefox-debugopt/xpcom/build/nsDebug.cpp:109)
PresShell::RetargetEventToParent(nsIView*, nsGUIEvent*, nsEventStatus*, int,
int&, nsIContent*) (/builds/trunk/mozilla/layout/base/nsPresShell.cpp:5942)
PresShell::HandleEvent(nsIView*, nsGUIEvent*, nsEventStatus*, int, int&)
(/builds/trunk/mozilla/layout/base/nsPresShell.cpp:6107)
nsViewManager::HandleEvent(nsView*, nsGUIEvent*, int)
(/builds/trunk/mozilla/view/src/nsViewManager.cpp:2512)
.L2454 (/builds/trunk/mozilla/view/src/nsViewManager.cpp:2246)
HandleEvent (/builds/trunk/mozilla/view/src/nsView.cpp:171)
nsCommonWidget::DispatchEvent(nsGUIEvent*, nsEventStatus&)
(/builds/trunk/mozilla/widget/src/gtk2/nsCommonWidget.cpp:219)
nsWindow::OnKeyPressEvent(_GtkWidget*, _GdkEventKey*)
(/builds/trunk/mozilla/widget/src/gtk2/nsWindow.cpp:1694)
key_press_event_cb (/builds/trunk/mozilla/widget/src/gtk2/nsWindow.cpp:3835)
UNKNOWN  (/usr/lib/libgtk-x11-2.0.so.0)
g_closure_invoke+0x000000F6  (/usr/lib/libgobject-2.0.so.0)
UNKNOWN  (/usr/lib/libgobject-2.0.so.0)
g_signal_emit_valist+0x000005B6  (/usr/lib/libgobject-2.0.so.0)
g_signal_emit+0x00000032  (/usr/lib/libgobject-2.0.so.0)
UNKNOWN  (/usr/lib/libgtk-x11-2.0.so.0)
gtk_window_propagate_key_event+0x000000C5  (/usr/lib/libgtk-x11-2.0.so.0)
UNKNOWN  (/usr/lib/libgtk-x11-2.0.so.0)
UNKNOWN  (/usr/lib/libgtk-x11-2.0.so.0)
UNKNOWN  (/usr/lib/libgobject-2.0.so.0)
g_closure_invoke+0x000000F6  (/usr/lib/libgobject-2.0.so.0)
UNKNOWN  (/usr/lib/libgobject-2.0.so.0)
g_signal_emit_valist+0x000005B6  (/usr/lib/libgobject-2.0.so.0)
g_signal_emit+0x00000032  (/usr/lib/libgobject-2.0.so.0)
UNKNOWN  (/usr/lib/libgtk-x11-2.0.so.0)
gtk_propagate_event+0x00000217  (/usr/lib/libgtk-x11-2.0.so.0)
gtk_main_do_event+0x000001B4  (/usr/lib/libgtk-x11-2.0.so.0)
UNKNOWN  (/usr/lib/libgdk-x11-2.0.so.0)
g_main_context_dispatch+0x000001CB  (/usr/lib/libglib-2.0.so.0)
UNKNOWN  (/usr/lib/libglib-2.0.so.0)
g_main_loop_run+0x000001A9  (/usr/lib/libglib-2.0.so.0)
gtk_main+0x000000B2  (/usr/lib/libgtk-x11-2.0.so.0)
nsAppShell::Run() (/builds/trunk/mozilla/widget/src/gtk2/nsAppShell.cpp:141)
nsAppStartup::Run()
(/builds/trunk/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:146)
XRE_main (/builds/trunk/mozilla/toolkit/xre/nsAppRunner.cpp:2324)
main (/builds/trunk/mozilla/browser/app/nsBrowserApp.cpp:62)
*** Bug 303726 has been marked as a duplicate of this bug. ***
This would indicate that the event is being handled by a pres shell that's
cached in history (the only time a prescontext is unhooked from its container).
 This should never be able to happen.  But, it's possible that from the time
split-window landed up until the fix for bug 298077 landed, that we were leaving
focus in the old document, which might explain this.

I'm not seeing this in talkback crash reports either, so David, can you confirm
whether you're still seeing this after the 298077 checkin?
I'm pretty sure it was after I pulled yesterday's bug 298077 checkin.  It
wouldn't show up in talkback summaries until tomorrow.
topcrasher, this ought to block
Flags: blocking1.8b4?
Assignee: events → bryner
Flags: blocking1.8b4? → blocking1.8b4+
bryner, looks like it is bfcache related?
Looks like bug 304003 has helped out a lot here.  This may be fixed now, watch
for talkback reports in builds > 080914.
I saw this again just now in a tree pulled at 22:30 PDT last night.
Keywords: topcrashtopcrash+
According to talkback, this is still crashing. Clearly 304003 did not fix this. 
Attached patch patchSplinter Review
This patch makes us be more aggressive about locating a container when using
bfcache.  While we keep the container disconnected for all other purposes, we
keep it around as a separate nsWeakPtr for the purpose of targetting events at
the parent PresShell.  As part of this, I made it bail if it could not find
anywhere to send the events.

Unfortunately, I've only managed to trigger the condition that causes the crash
once, and so I can't verify that this patch fixes the problem.
Attachment #193004 - Flags: review?(aaronleventhal)
Comment on attachment 193004 [details] [diff] [review]
patch

One question -- what can a container be other than an nsIDocShell? Why are we
always handing around nsISupports for containers? I only ever see them turned
into docshells.
Attachment #193004 - Flags: review?(aaronleventhal) → review+
Yeah, they're only ever docshells... someone over-modularizing I guess.  
Should clean that up for Gecko 1.9.
Attachment #193004 - Flags: superreview?(dbaron)
Attachment #193004 - Flags: approval1.8b4?
Severity: normal → critical
bryner, it looks like dbaron is on vacation until the 24th.  we might want to
see if we can find one or two other people to superreview in his stead.
Comment on attachment 193004 [details] [diff] [review]
patch

ugly, but sr=dbaron
Attachment #193004 - Flags: superreview?(dbaron) → superreview+
Attachment #193004 - Flags: approval1.8b4? → approval1.8b4+
checked in on trunk and branch
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Perhaps part of the reason I wasn't able to verify that the fix did anything...
you can't do_GetWeakReference on a nsWeakPtr.  I changed it to just pass in the
container as an nsWeakPtr.  Should fix the orange on balsa.
Attachment #193470 - Flags: superreview?(dbaron)
Attachment #193470 - Flags: review?(dbaron)
Attachment #193470 - Flags: superreview?(dbaron)
Attachment #193470 - Flags: superreview+
Attachment #193470 - Flags: review?(dbaron)
Attachment #193470 - Flags: review+
Attachment #193470 - Flags: approval1.8b4?
Attachment #193470 - Flags: approval1.8b4? → approval1.8b4+
(In reply to comment #16)
> Created an attachment (id=193470) [edit]
> fix assertions and other problems
> 
> Perhaps part of the reason I wasn't able to verify that the fix did anything...
> you can't do_GetWeakReference on a nsWeakPtr.  I changed it to just pass in the
> container as an nsWeakPtr.  Should fix the orange on balsa.

Checked in on the trunk and 1.8 branch.
verified, this hasn't shown up in talkback reports since 8/21
Status: RESOLVED → VERIFIED
Keywords: fixed1.8verified1.8
Crash Signature: [@ PresShell::RetargetEventToParent]
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.