One of the Linux topcrashes in Firefox 1.5.0.2 is a null pointer dereference in IM_get_input_context, which unconditionally dereferences owningWindow, which is null when we crash. The top of the stack is pretty constant between incidents although the source of the document viewer destruction varies (direct or in fastback cache): IM_get_input_context() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/widget/src/gtk2/nsWindow.cpp, line 4793] nsWindow::IMELoseFocus() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/widget/src/gtk2/nsWindow.cpp, line 4387] nsWindow::IMEDestroyContext() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/widget/src/gtk2/nsWindow.cpp, line 4358] nsWindow::Destroy() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/widget/src/gtk2/nsWindow.cpp, line 404] nsView::~nsView() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/view/src/nsView.cpp, line 267] nsIView::Destroy() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/view/src/nsView.cpp, line 304] nsFrame::Destroy() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/layout/generic/nsFrame.cpp, line 669] nsContainerFrame::Destroy() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/layout/generic/nsContainerFrame.cpp, line 169] nsFrameManager::Destroy() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/layout/base/nsFrameManager.cpp, line 298] PresShell::Destroy() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/layout/base/nsPresShell.cpp, line 1957] DocumentViewerImpl::Destroy() [/builds/tinderbox/Fx-Mozilla1.8.0/Linux_2.4.21-37.ELsmp_Depend/mozilla/layout/base/nsDocumentViewer.cpp, line 712]

taking. but what is cause of this regression?

This patch is not needed on Trunk. Because trunk already has this code.

I cannot reproduce this bug, but maybe the patch fixes this bug.

Oops. Sorry. I'm wrong.

Comment on attachment 219454 [details] [diff] [review] Patch rv1.1 for 1.8.x +katakai I'm also not seeing this problem on my environment, but the patch seems OK.

Uh, I found similar risk. We should fix this too.

Comment on attachment 219531 [details] [diff] [review] Patch rv1.0 for Trunk +katakai

Comment on attachment 219454 [details] [diff] [review] Patch rv1.1 for 1.8.x The code is fine, but can you explain why aArea can be null or owningWindow can be null?

Comment on attachment 219454 [details] [diff] [review] Patch rv1.1 for 1.8.x Actually, let's wait until we can verify that this fixes the real issue. Do we know anyone who can reproduce the crash here?

(In reply to comment #10) > (From update of attachment 219454 [details] [diff] [review] [edit]) > Actually, let's wait until we can verify that this fixes the real issue. Do we > know anyone who can reproduce the crash here? > No, I don't know. I don't see this report on bugzilla-jp, Japanese forums and Japanese blogs.

Oshima-san: Can you reproduce this bug?

Could it be a teardown ordering issue? Are the stacks something that commonly happens during window teardown?

dbaron: looks like it to me. I'm a bit afraid that checking in this patch will just make us crash somewhere else, or worse, corrupt memory instead of crashing.

This sounds like the sort of fix that truly needs the trunk-baking time to make sure there are't any bad regressions or side-effects. Too late for 1.8.0.4

(In reply to comment #15) > This sounds like the sort of fix that truly needs the trunk-baking time to make > sure there are't any bad regressions or side-effects. Too late for 1.8.0.4 We cannot test on Trunk, because the trunk doesn't have this code. We can test only on MOZILLA_1_8_BRANCH.

Boris has reported same report in bug 337036. Boris: Can you help us? See comment 10, comment 13 and comment 14.

So I see bug 337036 almost half the time when closing windows. Simple testcase: javascript:alert('aaa') Then click "ok". Half the time I crash (inside GTK); that's bug 337036 (which has a slightly different stack from this one). The other half, I get: (Gecko:12931): GLib-GObject-WARNING **: invalid cast from `(null)' to `GtkWidget' (Gecko:12931): GLib-GObject-WARNING **: invalid cast from `(null)' to `GObject' (Gecko:12931): GLib-GObject-CRITICAL **: file gobject.c: line 1642 (g_object_get_data): assertion `G_IS_OBJECT (object)' failed When I crash, mIMEData is false. So I wouldn't even hit the codepath the patch in this bug is changing. Furthermore, in IM_get_owning_window I have a non-null aArea and a non-null aArea->inner_window. But the call to get_gtk_widget_for_gdk_window crashes. So as far as I can tell, either bug 337036 is a different bug or this patch is wallpaper that doesn't fix the underlying issue.

(In reply to comment #18) > Half the time I crash (inside GTK); that's bug 337036 (which has a slightly > different stack from this one). Ah, right, sorry.

Can we get this bug landed on the 1.8 branch ("fixed1.8.1" state) and get some verification there that this doesn't cause the worse problems roc was worried about?

Has anyone yet confirmed that this patch actually fixes anything? Comment #18 suggests it does not. I don't think we should check in a patch that we're not sure actually fixes anything.

Too late to get an unbaked patch into 1.8.0.5 for a non-security issue.

Minusing for blocking1.8.1 because it doesn't look like we're going to be able to get a fix for this one, although it would be great if we could still get a patch.

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

Just a long shot: does the patch in bug 351225 fix it? (speculating that we destroy a view/window that has already been destroyed)

(In reply to comment #25) > Just a long shot: does the patch in bug 351225 fix it? > (speculating that we destroy a view/window that has already been destroyed) > I don't think so; we have received a report whose stacktrace looks quite similar from a user of 2.0.0.1. bug 351225 is verified1.8.1.1 so this is unlikely to be the same problem. You can take a look here: https://launchpad.net/ubuntu/+source/firefox/+bug/85627

For all intents and purposes, this crash is the #1 Linux topcrash in 2.0.0.1 (only crashes in libc.so.6 and Flash outnumber it). Renominating in hopes that someone has time to take a second look.

Not going to block 1.8.1.3 for this, this is a very short cycle with no time for baking. Moving nom over to 1.8.1.4, we probably want to take this early.

We should get this fixed -- the small percentage of our users who use an IM (5%? 10% at most?) have managed to generate the #2 linux top crash. roc: is there a way to track down your concerns in comment 14?

roc: see comment 29.

Comment on attachment 219454 [details] [diff] [review] Patch rv1.1 for 1.8.x OK. I'm unconvinced this will fix anything but it obviously won't make things worse, so let's land this on the 1.8.1 branch and see if that affects anything. What we really need here, though, is someone who can reproduce this crash reliably. If it's such a huge crasher we should be able to find such a person...

Comment on attachment 219454 [details] [diff] [review] Patch rv1.1 for 1.8.x Thank you roc. Let's land it.

Comment on attachment 219454 [details] [diff] [review] Patch rv1.1 for 1.8.x approved for 1.8.1.4, a=dveditz for release-drivers

checked-in to 1.8 branch.

Verified with Simple testcase: javascript:alert('aaa') Then click "ok" on Linux FC5 with 2.0.0.4 rc2

i never saw this crash with the simple testcase mentioned. Hope this fix does not just split this issue in crashes at multiple places.