451341 - xulrunner crashes when fails to find the window on which to change the cursor

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P2)

Description

[Tomeu Vizoso]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.1) Gecko/2008072820 Firefox/3.0.1
Build Identifier: 

In http://mxr.mozilla.org/seamonkey/source/widget/src/gtk2/nsWindow.cpp#923 :

914 NS_IMETHODIMP
915 nsWindow::SetCursor(nsCursor aCursor)
916 {
917     // if we're not the toplevel window pass up the cursor request to
918     // the toplevel window to handle it.
919     if (!mContainer && mDrawingarea) {
920         GtkWidget *widget =
921             get_gtk_widget_for_gdk_window(mDrawingarea->inner_window);
922         nsWindow *window = get_window_for_gtk_widget(widget);
923         return window->SetCursor(aCursor);
924     }

get_window_for_gtk_widget() can return NULL in some circumstances, we should guard against this with something similar to this (from nsWindow::SetModal(PRBool aModal)):

547         if (!parent)
548             return NS_ERROR_FAILURE;


Reproducible: Couldn't Reproduce




An user of Sugar has reported it, but I couldn't reproduce it.

Comment 1

[Martin Bähr]

Attachment: backtrace

the sugar bug report is at http://dev.laptop.org/ticket/8056

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Karl, can you roll a patch here?

Comment 3

[Karl Tomlinson (:karlt)]

Yes, get_window_for_gtk_widget() can return NULL in some circumstances, but I
don't think it should in this case.  It does look like it has returned
something wrong, but I think this could be a symptom of a more complicated
problem.

All MozDrawingarea inner_windows should have a parent MozContainer widget
(while the MozDrawingarea still exists at least), and all MozContainer widgets
should have an nsWindow (while the descendant nsWindow has a MozDrawingarea at
least).

Martin, thank you for the backtrace.  Would you be able to print the values of
mDrawingarea->inner_window, widget, and window, please?  That might help
narrow down where the problem is.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: clear mozdrawingarea from GdkWindow and add asserts [checked-in]

Correctly clear the "mozdrawingarea" from the data on the GdkWindows in case
something is grabbing a reference through nsWindow::GetNativeData() and then
using it as a parent to try to create a child nsWindow after the parent
nsWindow has been destroyed.

Add an assert to check that the MozContainer stored on MozDrawingarea's
GdkWindows remains valid through a reparent.

Add asserts that the parent MozContainer can be found and that it has an
nsWindow.

This is not likely to directly fix the problem, but will catch some possible
situations where the nsWindow and the embedding app won't interact well.

Comment 5

[Karl Tomlinson (:karlt)]

(In reply to comment #3)
> Martin, thank you for the backtrace.  Would you be able to print the values of
> mDrawingarea->inner_window, widget, and window, please?

BTW, sometimes gdb seems to need "this->mDrawingarea->inner_window".

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 335112 [details] [diff] [review]
clear mozdrawingarea from GdkWindow and add asserts [checked-in]

pushed:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/614a808f40a3

I don't expect this to fix the problem reported here, but it might help us catch some possible causes.
I'll leave this bug open in the hope that we might find the real cause of the problem.

Comment 7

[Karl Tomlinson (:karlt)]

After my 4 year daughter had been testing attachment 362355 [details] [diff] [review] for bug 478519,
this related shutdown crash was produced:

#5  0x00007fcb3e09333c in IA__g_type_check_instance_cast ()
   from /usr/lib64/libgobject-2.0.so.0
#6  0x00007fcb3462fe0e in get_gtk_widget_for_gdk_window (window=0x2a77dd0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:5109
#7  0x00007fcb34632386 in nsWindow::GetMozContainerWidget (this=0x38636d0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:4656
#8  0x00007fcb346324da in nsWindow::GetToplevelWidget (this=0x38636d0,
    aWidget=0x7fff49257848)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:4645
#9  0x00007fcb3463ae8d in nsWindow::IsVisible (this=0x38636d0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:837
#10 0x00007fcb3463b568 in nsWindow::Invalidate (this=0x38636d0,
    aRect=@0x7fff492579e0, aIsSynchronous=0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:1645
#11 0x00007fcb30052ca6 in nsViewManager::UpdateWidgetArea (this=0x36f0450,
    aWidgetView=0x2cde5a0, aDamagedRegion=@0x7fff49257b20,
(gdb) f 6
#6  0x00007fcb3462fe0e in get_gtk_widget_for_gdk_window (window=0x2a77dd0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:5109
(gdb) up
#7  0x00007fcb34632386 in nsWindow::GetMozContainerWidget (this=0x38636d0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:4656
(gdb) p *this
$1 = (nsChildWindow) {<nsWindow> = {<nsBaseWidget> = {<nsIWidget> = {<nsISupports> = {_vptr.nsISupports = 0x7fcb348ba510}, mFirstChild = {mRawPtr = 0x0},
        mLastChild = 0x0, mNextSibling = {mRawPtr = 0x0}, mPrevSibling = 0x0},
      mRefCnt = {mValue = 2}, _mOwningThread = {mThread = 0x1abba20},
      mClientData = 0x3b2c3f0, mEventCallback = 0x7fcb3004a3e4 <HandleEvent>,
      mContext = 0x2dbb7c0, mToolkit = 0x1fec5c0, mEventListener = 0x0,
      mBackground = 33157480, mForeground = 0, mCursor = eCursor_standard,
      mWindowType = eWindowType_child, mBorderStyle = eBorderStyle_default,
      mIsShiftDown = 0 '\0', mIsControlDown = 0 '\0', mIsAltDown = 0 '\0',
      mIsDestroying = 0 '\0', mOnDestroyCalled = 0 '\0', mBounds = {x = 0,
        y = 0, width = 1672, height = 872}, mOriginalBounds = 0x0,
      mZIndex = 0, mSizeMode = nsSizeMode_Normal,
      static mLastRollup = 0x0}, <nsSupportsWeakReference> = {<nsISupportsWeakReference> = {<nsISupports> = {
          _vptr.nsISupports = 0x7fcb348ba8b0}, <No data fields>},
      mProxy = 0x0}, mOldFocusWindow = 0,
    static mLastButtonPressTime = 989732800,
    static mLastButtonReleaseTime = 989733000, mIMEData = 0x0,
    static sAccessibilityEnabled = 0, mParent = {mRawPtr = 0x3148170},
    mIsTopLevel = 0 '\0', mIsDestroyed = 0 '\0', mNeedsResize = 0 '\0',
    mNeedsMove = 0 '\0', mListenForResizes = 0 '\0', mIsShown = 1 '\001',
    mNeedsShow = 0 '\0', mEnabled = 1 '\001', mCreated = 1 '\001',
    mPlaced = 1 '\001', mPreferredWidth = 0, mPreferredHeight = 0,
    mShell = 0x0, mContainer = 0x0, mDrawingarea = 0x2dc05f0,
    mWindowGroup = 0x0, mContainerGotFocus = 0, mContainerLostFocus = 0,
    mContainerBlockFocus = 0, mIsVisible = 1, mRetryPointerGrab = 0,
    mActivatePending = 0, mRetryKeyboardGrab = 0, mTransientParent = 0x0,
    mSizeState = 0, mPluginType = nsWindow::PluginType_NONE,
    mTransparencyBitmapWidth = 0, mTransparencyBitmapHeight = 0,
    mThebesSurface = {mRawPtr = 0x2a61bc0}, mRootAccessible = {mRawPtr = 0x0},
    static gsGtkCursorCache = <optimized out>, mIsTransparent = 0,
    mTransparencyBitmap = 0x0, static mLastDragMotionWindow = 0x0,
    mDragMotionWidget = 0x0, mDragMotionContext = 0x0, mDragMotionX = 0,
    mDragMotionY = 0, mDragMotionTime = 0, mDragMotionTimerID = 0,
    mDragLeaveTimer = {mRawPtr = 0x0}, mLastMotionPressure = 0,
    static sIsDraggingOutOf = 0, mKeyDownFlags = {0, 0, 0, 0, 0, 0, 0,
      0}}, <No data fields>}
(gdb) down
#6  0x00007fcb3462fe0e in get_gtk_widget_for_gdk_window (window=0x2a77dd0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:5109
(gdb) p user_data
$2 = (gpointer) 0x2148510
(gdb) p *window
$3 = {parent_instance = {g_type_instance = {g_class = 0x1adf8d0},
    ref_count = 1, qdata = 0x3858da0}}
(gdb) p /x *(GObject*)user_data
$7 = {g_type_instance = {g_class = 0x2376260}, ref_count = 0x2aae610,
  qdata = 0x0}
(gdb) p /x *(GtkWidget*)user_data
$8 = {object = {parent_instance = {g_type_instance = {g_class = 0x2376260},
      ref_count = 0x2aae610, qdata = 0x0}, flags = 0x10e00},
  private_flags = 0x3400, state = 0x0, saved_state = 0x0, name = 0x0,
  style = 0x0, requisition = {width = 0x0, height = 0x0}, allocation = {
    x = 0x0, y = 0x0, width = 0x1, height = 0x1}, window = 0x0, parent = 0x0}

GtkWidget *
get_gtk_widget_for_gdk_window(GdkWindow *window)
{
    gpointer user_data = NULL;
    gdk_window_get_user_data(window, &user_data);
    if (!user_data)
        return NULL;

    return GTK_WIDGET(user_data);
}

So the window still looks valid, but the MozContainer widget does not.

Comment 8

[Karl Tomlinson (:karlt)]

On adding these lines to moz_drawingarea_finalize,

     drawingarea = MOZ_DRAWINGAREA(object);
 
+#ifdef DEBUG
+    gpointer user_data = NULL;
+    gdk_window_get_user_data(drawingarea->inner_window, &user_data);
+#endif
 
     gdk_window_set_user_data(drawingarea->inner_window, NULL);

and using a breakpoint conditional on "((GObject*)user_data)->ref_count == 0 ||
((GObject*)user_data)->ref_count > 20" I caught a similar situation on shutdown (after a bit of browsing):

(firefox-bin:24284): GLib-GObject-WARNING **: invalid uninstantiatable type `<unknown>' in cast to `GtkWidget'
###!!! ASSERTION: Lost our MozContainer: 'IS_MOZ_CONTAINER(owningWidget)', file /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp, line 4667

(firefox-bin:24284): Gtk-CRITICAL **: gtk_widget_get_toplevel: assertion `GTK_IS_WIDGET (widget)' failed

Breakpoint 21, moz_drawingarea_finalize (object=0x27fa410)
    at /home/karl/moz/dev/widget/src/gtk2/mozdrawingarea.c:201

#0  moz_drawingarea_finalize (object=0x27fa410)
    at /home/karl/moz/dev/widget/src/gtk2/mozdrawingarea.c:201
#1  0x00007f1c4b1e757a in IA__g_object_unref ()
   from /usr/lib64/libgobject-2.0.so.0
#2  0x00007f1c417a8e28 in nsWindow::Destroy (this=0x27fb7c0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:731
#3  0x00007f1c3d1b6e87 in ~nsView (this=0x27fb710)
    at /home/karl/moz/dev/view/src/nsView.cpp:271
#4  0x00007f1c3d1b9593 in ~nsScrollPortView (this=0x27fb710)
    at /home/karl/moz/dev/view/src/nsScrollPortView.cpp:107
#5  0x00007f1c3d1b475f in nsIView::Destroy (this=0x27fb710)
    at /home/karl/moz/dev/view/src/nsView.cpp:313
#6  0x00007f1c3d1b67a9 in ~nsView (this=0x27f4490)
    at /home/karl/moz/dev/view/src/nsView.cpp:223
#7  0x00007f1c3d1b475f in nsIView::Destroy (this=0x27f4490)
    at /home/karl/moz/dev/view/src/nsView.cpp:313
#8  0x00007f1c3cd0f562 in nsFrame::Destroy (this=0xbb54e0)
    at /home/karl/moz/dev/layout/generic/nsFrame.cpp:500
#9  0x00007f1c3cd7c546 in nsSplittableFrame::Destroy (this=0xbb54e0)
    at /home/karl/moz/dev/layout/generic/nsSplittableFrame.cpp:73
#10 0x00007f1c3ccf92f4 in nsContainerFrame::Destroy (this=0xbb54e0)
    at /home/karl/moz/dev/layout/generic/nsContainerFrame.cpp:305
#11 0x00007f1c3cd28c6a in nsHTMLScrollFrame::Destroy (this=0xbb54e0)
    at /home/karl/moz/dev/layout/generic/nsGfxScrollFrame.cpp:157
#12 0x00007f1c3cd165be in nsFrameList::DestroyFrames (this=0xbb5198)
    at /home/karl/moz/dev/layout/generic/nsFrameList.cpp:67
#13 0x00007f1c3ccf9179 in nsContainerFrame::Destroy (this=0xbb5140)
    at /home/karl/moz/dev/layout/generic/nsContainerFrame.cpp:263
#14 0x00007f1c3cd97fd0 in ViewportFrame::Destroy (this=0xbb5140)
    at /home/karl/moz/dev/layout/generic/nsViewportFrame.cpp:68
#15 0x00007f1c3cc928e7 in nsFrameManager::Destroy (this=0xbb3038)
    at /home/karl/moz/dev/layout/base/nsFrameManager.cpp:292
#16 0x00007f1c3ccbe069 in PresShell::Destroy (this=0xbb3000)
    at /home/karl/moz/dev/layout/base/nsPresShell.cpp:1706
#17 0x00007f1c3cc7987a in DocumentViewerImpl::DestroyPresShell (this=0x279e470)
    at /home/karl/moz/dev/layout/base/nsDocumentViewer.cpp:4203
#18 0x00007f1c3cc81473 in DocumentViewerImpl::Destroy (this=0x279e470)
    at /home/karl/moz/dev/layout/base/nsDocumentViewer.cpp:1526

(gdb) p /x *(MozContainer*)user_data
$137 = {container = {widget = {object = {parent_instance = {g_type_instance = {
            g_class = 0xea0860}, ref_count = 0x49ba1ab0, qdata = 0x0}, 
        flags = 0x10e00}, private_flags = 0x3400, state = 0x0, 
      saved_state = 0x0, name = 0x0, style = 0x0, requisition = {width = 0x0, 
        height = 0x0}, allocation = {x = 0x0, y = 0x0, width = 0x1, 
        height = 0x1}, window = 0x0, parent = 0x0}, focus_child = 0x0, 
    border_width = 0x0, need_resize = 0x0, resize_mode = 0x2, 
    reallocate_redraws = 0x0, has_focus_chain = 0x0}, children = 0xb0}

Comment 9

[Karl Tomlinson (:karlt)]

(In reply to comment #3)
> All MozDrawingarea inner_windows should have a parent MozContainer widget
> (while the MozDrawingarea still exists at least)

It seems that we can get into a situation where the MozContainer widget is not
valid when an nsWindow of type eWindowType_child is created with a native
GdkWindow parent (instead of an nsWindow parent), and the nsWindow
corresponding to the parent GdkWindow and MozContainer gets destroyed before
the new nsWindow of type eWindowType_child.

The MozContainer for the new nsWindow's MozDrawingarea is obtained from the
parent GdkWindow, but the new nsWindow is not registered as a child of the
parent GdkWindow's nsWindow.  If the new nsWindow were registered as a child
then it would be destroyed before the parent was destroyed, but, as it is not
registered, the parent nsWindow can be destroyed first, destroying the
MozContainer before the child nsWindow's MozDrawingarea, leaving a dangling
pointer to the MozContainer.

I haven't (yet) been able to determine why unconnected parent nsWindows are
destroyed before their indirect child nsWindows, but, if this situation cannot
be excluded, possible options for resolving include.

1) Register the new nsWindow as a child of the parent GdkWindow's nsWindow.

   I don't know what assumptions are made about the nsIWidget hierarchy, and
   so I'm not sure what effects this might have.  Other platforms don't put
   nsWindows into the hierarchy when created with a native parent.

2) Create a new MozContainer widget for the new nsWindow.

   This seems a bit of a waste of MozContainer widgets.

   When the parent GdkWindow is destroyed, the new nsWindow's GdkWindows are
   also "destroy"ed (but not unref'ed), so adding a MozContainer doesn't
   lengthen the functional life of the nsWindow.  There is little gain from
   having extra MozContainers.

3) Check whether the (child) GdkWindow is destroyed before using it to look up
   the MozContainer.

   This can be done by casting the GdkWindow* to a struct GdkWindowObject*
   which has a "destroyed" field.  We already do this in
   nsWindow::SetupPluginPort, and the struct and casting macro are defined in
   gdkwindow.h, but I don't see any documentation on GdkWindowObject.

4) Add weak reference callbacks to the MozContainer for each of its
   MozDrawingareas, so the MozDrawingareas get notified when their
   MozContainer is disposed.

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: make the pointer from MozDrawingarea to MozContainer a weak reference

This is option 4 from comment 9.  I still need to make another patch adding
NULL checks in the places where these weak references are accessed.

The patch also removes MozDrawingarea::parent, which is not used, so that
there is no need to fix up dangling pointers there.

The toplevel nsWindow first gets destroyed on an event (when clicking the
frame's close box at least), then the eWindowType_child nsWindows with weak
references to the MozContainer on the toplevel nsWindow get destroyed on
GC/shutdown.

toplevel nsWindow:

#0  nullify_widget_pointers (data=0x3066bb0, widget=0x1a3b040)
    at /home/karl/moz/dev/widget/src/gtk2/mozdrawingarea.c:134
#1  0x00007fd24e602e98 in weak_refs_notify ()
   from /usr/lib64/libgobject-2.0.so.0
#2  0x00007fd24e13c24b in IA__g_datalist_id_set_data_full ()
   from /usr/lib64/libglib-2.0.so.0
#3  0x00007fd24e603972 in IA__g_object_run_dispose ()
   from /usr/lib64/libgobject-2.0.so.0
#4  0x00007fd24f675d5b in gtk_container_destroy ()
   from /usr/lib64/libgtk-x11-2.0.so.0
#5  0x00007fd24e601a34 in IA__g_closure_invoke ()
   from /usr/lib64/libgobject-2.0.so.0
#6  0x00007fd24e61520a in signal_emit_unlocked_R ()
   from /usr/lib64/libgobject-2.0.so.0
#7  0x00007fd24e6166a8 in IA__g_signal_emit_valist ()
   from /usr/lib64/libgobject-2.0.so.0
#8  0x00007fd24e616a9f in IA__g_signal_emit ()
   from /usr/lib64/libgobject-2.0.so.0
#9  0x00007fd24f711e6b in gtk_object_dispose ()
   from /usr/lib64/libgtk-x11-2.0.so.0
#10 0x00007fd24e603972 in IA__g_object_run_dispose ()
   from /usr/lib64/libgobject-2.0.so.0
#11 0x00007fd244bc4fab in nsWindow::Destroy (this=0x1a394d0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:736
#12 0x00007fd244bc538a in ~nsWindow (this=0x1a394d0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:460
#13 0x00007fd244bf5fa5 in nsBaseWidget::Release (this=0x1a394d0)
    at /home/karl/moz/dev/widget/src/xpwidgets/nsBaseWidget.cpp:69
#14 0x00007fd244bc51f1 in nsWindow::Release (this=0x1a394d0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:474
#15 0x00007fd244bc6962 in ~nsRefPtr (this=0x7fff597e1000)
    at ../../../dist/include/xpcom/nsAutoPtr.h:956
#16 0x00007fd244bba4be in delete_event_cb (widget=0x1a399a0, event=0x2767990)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:5335
#17 0x00007fd24f6f64a5 in _gtk_marshal_BOOLEAN__BOXED ()
   from /usr/lib64/libgtk-x11-2.0.so.0
#18 0x00007fd24e601a34 in IA__g_closure_invoke ()
   from /usr/lib64/libgobject-2.0.so.0
#19 0x00007fd24e614b22 in signal_emit_unlocked_R ()
   from /usr/lib64/libgobject-2.0.so.0
#20 0x00007fd24e6163c2 in IA__g_signal_emit_valist ()
   from /usr/lib64/libgobject-2.0.so.0
#21 0x00007fd24e616a9f in IA__g_signal_emit ()
   from /usr/lib64/libgobject-2.0.so.0
#22 0x00007fd24f7e79dd in gtk_widget_event_internal ()
   from /usr/lib64/libgtk-x11-2.0.so.0
#23 0x00007fd24f6f1fbb in IA__gtk_main_do_event ()
   from /usr/lib64/libgtk-x11-2.0.so.0
#24 0x00007fd24f157b0e in gdk_event_dispatch ()
   from /usr/lib64/libgdk-x11-2.0.so.0
#25 0x00007fd24e14d275 in IA__g_main_context_dispatch ()
   from /usr/lib64/libglib-2.0.so.0
#26 0x00007fd24e1503e7 in g_main_context_iterate ()
   from /usr/lib64/libglib-2.0.so.0
#27 0x00007fd24e150918 in IA__g_main_context_iteration ()
   from /usr/lib64/libglib-2.0.so.0
#28 0x00007fd244bcc638 in nsAppShell::ProcessNextNativeEvent (this=0x14cd810, 
    mayWait=1) at /home/karl/moz/dev/widget/src/gtk2/nsAppShell.cpp:144
#29 0x00007fd244bf3744 in nsBaseAppShell::DoProcessNextNativeEvent (
    this=0x14cd810, mayWait=1)
    at /home/karl/moz/dev/widget/src/xpwidgets/nsBaseAppShell.cpp:151
#30 0x00007fd244bf3c69 in nsBaseAppShell::OnProcessNextEvent (this=0x14cd810, 
    thr=0x13f8160, mayWait=1, recursionDepth=0)
    at /home/karl/moz/dev/widget/src/xpwidgets/nsBaseAppShell.cpp:296
#31 0x00007fd250463353 in nsThread::ProcessNextEvent (this=0x13f8160, 
    mayWait=1, result=0x7fff597e1a5c)
    at /home/karl/moz/dev/xpcom/threads/nsThread.cpp:497
#32 0x00007fd2503f4708 in NS_ProcessNextEvent_P (thread=0x13f8160, mayWait=1)
    at nsThreadUtils.cpp:230
#33 0x00007fd244bf3e84 in nsBaseAppShell::Run (this=0x14cd810)
    at /home/karl/moz/dev/widget/src/xpwidgets/nsBaseAppShell.cpp:170
#34 0x00007fd2429e8724 in nsAppStartup::Run (this=0x1564100)
    at /home/karl/moz/dev/toolkit/components/startup/src/nsAppStartup.cpp:192
#35 0x00007fd251396780 in XRE_main (argc=3, argv=0x7fff597e2448, 
    aAppData=0x13add20) at /home/karl/moz/dev/toolkit/xre/nsAppRunner.cpp:3216
#36 0x00000000004012cb in main (argc=3, argv=0x7fff597e2448)
    at /home/karl/moz/dev/browser/app/nsBrowserApp.cpp:156

eWindowType_child nsWindow:

#0  moz_drawingarea_finalize (object=0x2f71740)
    at /home/karl/moz/dev/widget/src/gtk2/mozdrawingarea.c:218
#1  0x00007fd24e60357a in IA__g_object_unref ()
   from /usr/lib64/libgobject-2.0.so.0
#2  0x00007fd244bc4f7c in nsWindow::Destroy (this=0x2f6dbe0)
    at /home/karl/moz/dev/widget/src/gtk2/nsWindow.cpp:731
#3  0x00007fd2405d2e87 in ~nsView (this=0x2f6db30)
    at /home/karl/moz/dev/view/src/nsView.cpp:271
#4  0x00007fd2405d5593 in ~nsScrollPortView (this=0x2f6db30)
    at /home/karl/moz/dev/view/src/nsScrollPortView.cpp:107
#5  0x00007fd2405d075f in nsIView::Destroy (this=0x2f6db30)
    at /home/karl/moz/dev/view/src/nsView.cpp:313
#6  0x00007fd2405d27a9 in ~nsView (this=0x2f890a0)
    at /home/karl/moz/dev/view/src/nsView.cpp:223
#7  0x00007fd2405d075f in nsIView::Destroy (this=0x2f890a0)
    at /home/karl/moz/dev/view/src/nsView.cpp:313
#8  0x00007fd24012b562 in nsFrame::Destroy (this=0x2edcac0)
    at /home/karl/moz/dev/layout/generic/nsFrame.cpp:500
#9  0x00007fd240198546 in nsSplittableFrame::Destroy (this=0x2edcac0)
    at /home/karl/moz/dev/layout/generic/nsSplittableFrame.cpp:73
#10 0x00007fd2401152f4 in nsContainerFrame::Destroy (this=0x2edcac0)
    at /home/karl/moz/dev/layout/generic/nsContainerFrame.cpp:305
#11 0x00007fd240144c6a in nsHTMLScrollFrame::Destroy (this=0x2edcac0)
    at /home/karl/moz/dev/layout/generic/nsGfxScrollFrame.cpp:157
#12 0x00007fd2401325be in nsFrameList::DestroyFrames (this=0x2edc778)
    at /home/karl/moz/dev/layout/generic/nsFrameList.cpp:67
#13 0x00007fd240115179 in nsContainerFrame::Destroy (this=0x2edc720)
    at /home/karl/moz/dev/layout/generic/nsContainerFrame.cpp:263
#14 0x00007fd2401b3fd0 in ViewportFrame::Destroy (this=0x2edc720)
    at /home/karl/moz/dev/layout/generic/nsViewportFrame.cpp:68
#15 0x00007fd2400ae8e7 in nsFrameManager::Destroy (this=0x2eda148)
    at /home/karl/moz/dev/layout/base/nsFrameManager.cpp:292
#16 0x00007fd2400da069 in PresShell::Destroy (this=0x2eda110)
    at /home/karl/moz/dev/layout/base/nsPresShell.cpp:1706
#17 0x00007fd24009587a in DocumentViewerImpl::DestroyPresShell (this=0x2eb5010)
    at /home/karl/moz/dev/layout/base/nsDocumentViewer.cpp:4203
#18 0x00007fd24009d473 in DocumentViewerImpl::Destroy (this=0x2eb5010)
    at /home/karl/moz/dev/layout/base/nsDocumentViewer.cpp:1526
#19 0x00007fd23f8e7d30 in ~nsSHEntry (this=0x1a36660)
    at /home/karl/moz/dev/docshell/shistory/src/nsSHEntry.cpp:163
#20 0x00007fd23f8e7f82 in nsSHEntry::Release (this=0x1a36660)
    at /home/karl/moz/dev/docshell/shistory/src/nsSHEntry.cpp:182
#21 0x00007fd244ea4f7a in XPCJSRuntime::GCCallback (cx=0x1a41ec0, 
    status=JSGC_END)
    at /home/karl/moz/dev/js/src/xpconnect/src/xpcjsruntime.cpp:781
#22 0x00007fd2405e5cab in DOMGCCallback (cx=0x1a41ec0, status=JSGC_END)
    at /home/karl/moz/dev/dom/src/base/nsJSEnvironment.cpp:3591
#23 0x00007fd244e7431d in XPCCycleCollectGCCallback (cx=0x1a41ec0, 
    status=JSGC_END)
    at /home/karl/moz/dev/js/src/xpconnect/src/nsXPConnect.cpp:412
#24 0x00007fd2510775e7 in js_GC (cx=0x1a41ec0, gckind=GC_NORMAL)
    at /home/karl/moz/dev/js/src/jsgc.cpp:3841
#25 0x00007fd251040943 in js_DestroyContext (cx=0x1a41ec0, mode=JSDCM_FORCE_GC)
    at /home/karl/moz/dev/js/src/jscntxt.cpp:542
#26 0x00007fd2510262b4 in JS_DestroyContext (cx=0x1a41ec0)
    at /home/karl/moz/dev/js/src/jsapi.cpp:1082
#27 0x00007fd244e709a7 in nsXPConnect::ReleaseJSContext (this=0x14d0d10, 
    aJSContext=0x1a41ec0, noGC=0)
    at /home/karl/moz/dev/js/src/xpconnect/src/nsXPConnect.cpp:2074
#28 0x00007fd2405ecc7b in nsJSContext::Unlink (this=0x1a41e60)
    at /home/karl/moz/dev/dom/src/base/nsJSEnvironment.cpp:1319
#29 0x00007fd2405ecd13 in nsJSContext::cycleCollection::Unlink (
    this=0x7fd240fc0668, p=0x1a41e60)
    at /home/karl/moz/dev/dom/src/base/nsJSEnvironment.cpp:1330
#30 0x00007fd25047a775 in nsCycleCollector::CollectWhite (this=0x7fd25162c010)
    at /home/karl/moz/dev/xpcom/base/nsCycleCollector.cpp:1666
#31 0x00007fd25047a84d in nsCycleCollector::FinishCollection (
    this=0x7fd25162c010)
    at /home/karl/moz/dev/xpcom/base/nsCycleCollector.cpp:2440
#32 0x00007fd25047a8ce in nsCycleCollector_finishCollection ()
    at /home/karl/moz/dev/xpcom/base/nsCycleCollector.cpp:2922
#33 0x00007fd244e742fb in XPCCycleCollectGCCallback (cx=0x1932d50, 
    status=JSGC_END)
    at /home/karl/moz/dev/js/src/xpconnect/src/nsXPConnect.cpp:404
#34 0x00007fd2510775e7 in js_GC (cx=0x1932d50, gckind=GC_NORMAL)
    at /home/karl/moz/dev/js/src/jsgc.cpp:3841
#35 0x00007fd25102274f in JS_GC (cx=0x1932d50)
    at /home/karl/moz/dev/js/src/jsapi.cpp:2494
#36 0x00007fd244e736f0 in nsXPConnect::Collect (this=0x14d0d10)
    at /home/karl/moz/dev/js/src/xpconnect/src/nsXPConnect.cpp:478
#37 0x00007fd25047ae35 in nsCycleCollector::Collect (this=0x7fd25162c010, 
    aTryCollections=1)
    at /home/karl/moz/dev/xpcom/base/nsCycleCollector.cpp:2256
#38 0x00007fd25047af21 in nsCycleCollector_collect ()
    at /home/karl/moz/dev/xpcom/base/nsCycleCollector.cpp:2904
#39 0x00007fd2405e6a30 in nsJSContext::CC ()
    at /home/karl/moz/dev/dom/src/base/nsJSEnvironment.cpp:3411
#40 0x00007fd2405e6ba3 in nsJSContext::CCIfUserInactive ()
    at /home/karl/moz/dev/dom/src/base/nsJSEnvironment.cpp:3498
#41 0x00007fd2405e6d8d in GCTimerFired (aTimer=0x1e39b20, aClosure=0x0)
    at /home/karl/moz/dev/dom/src/base/nsJSEnvironment.cpp:3519
#42 0x00007fd250469490 in nsTimerImpl::Fire (this=0x1e39b20)
    at /home/karl/moz/dev/xpcom/threads/nsTimerImpl.cpp:428
#43 0x00007fd2504696ae in nsTimerEvent::Run (this=0x7fd23800d420)
    at /home/karl/moz/dev/xpcom/threads/nsTimerImpl.cpp:520
#44 0x00007fd25046343c in nsThread::ProcessNextEvent (this=0x13f8160, 
    mayWait=1, result=0x7fff597e17dc)
    at /home/karl/moz/dev/xpcom/threads/nsThread.cpp:510
#45 0x00007fd2503f4708 in NS_ProcessNextEvent_P (thread=0x13f8160, mayWait=1)
    at nsThreadUtils.cpp:230
#46 0x00007fd250463868 in nsThread::Shutdown (this=0x192cff0)
    at /home/karl/moz/dev/xpcom/threads/nsThread.cpp:465
#47 0x00007fd25047dda5 in NS_InvokeByIndex_P (that=0x192cff0, methodIndex=6, 
    paramCount=0, params=0x0)
    at /home/karl/moz/dev/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_linux.cpp:208
#48 0x00007fd25046d2ff in nsProxyObjectCallInfo::Run (this=0x7fd238004310)
    at /home/karl/moz/dev/xpcom/proxy/src/nsProxyEvent.cpp:181
#49 0x00007fd25046343c in nsThread::ProcessNextEvent (this=0x13f8160, 
    mayWait=1, result=0x7fff597e1a5c)
    at /home/karl/moz/dev/xpcom/threads/nsThread.cpp:510
#50 0x00007fd2503f4708 in NS_ProcessNextEvent_P (thread=0x13f8160, mayWait=1)
    at nsThreadUtils.cpp:230
#51 0x00007fd244bf3e84 in nsBaseAppShell::Run (this=0x14cd810)
    at /home/karl/moz/dev/widget/src/xpwidgets/nsBaseAppShell.cpp:170
#52 0x00007fd2429e8724 in nsAppStartup::Run (this=0x1564100)
    at /home/karl/moz/dev/toolkit/components/startup/src/nsAppStartup.cpp:192
#53 0x00007fd251396780 in XRE_main (argc=3, argv=0x7fff597e2448, 
    aAppData=0x13add20) at /home/karl/moz/dev/toolkit/xre/nsAppRunner.cpp:3216
#54 0x00000000004012cb in main (argc=3, argv=0x7fff597e2448)
    at /home/karl/moz/dev/browser/app/nsBrowserApp.cpp:156

Comment 11

[Karl Tomlinson (:karlt)]

Attachment: NULL checks on MozContainer widget pointers

(with a little code cleanup)

Comment 12

[Karl Tomlinson (:karlt)]

Attachment: NULL checks on MozContainer widget pointers v1.1

The assertion in attachment 363044 [details] [diff] [review] wasn't valid: OnButtonPressEvent collects events on the nsWindow associated with event's window, while GTK collects the event on the widget with the grab (and provides that widget).  Given that the widget that GTK provides is different to the nsWindow's container widget, I'm not sure that the container widget still exists.

Comment 13

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/f34fb90675e7
http://hg.mozilla.org/mozilla-central/rev/2f2653c7c0e3

Comment 14

[Karl Tomlinson (:karlt)]

I think it would make sense to land these patches on 1.9.1.
I don't know exactly how we might get into the situation of dereferencing these dangling pointers, but better to be safe than sorry.

Comment 15

[XiaoXiaoHU]

Dear roc, could you help to check the progress of landing this patch on 1.9.1? Thanks.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

These patches can go to 1.9.1. When that happens we can check in your patch.

Comment 17

[Karl Tomlinson (:karlt)]

Requesting blocking1.9.0 for bug 484458.

Comment 18

[Karl Tomlinson (:karlt)]

Attachment: collective patch for 1.9.0

This is essentially all 3 patches here combined, but IMEComposeStart and IMEComposeText are a little different on 1.9.0.

Attachment 335112 [details] [diff] would be enough to fix what I saw in bug 484458, but the other changes fix a very similar issue.

Attachment 335112 [details] [diff] cleans up pointers on GdkWindows to nsWindows and MozDrawingareas.  The other attachments clean up pointers on GdkWindows to MozContainers.

Comment 19

[Karl Tomlinson (:karlt)]

Attachment: collective patch for 1.9.0

(Previous version was missing some files.)

Comment 20

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0b1ef1d8d913
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/93029b9670bc

Comment 21

[Samuel Sidler (old account; do not CC)]

Is there a test for this? Would be good to get a test before we commit to the patch for 1.9.0...

Comment 22

[Samuel Sidler (old account; do not CC)]

QA would like tests before approving this for 1.9.0. And, really, it probably needed them before 1.9.1 approval...

Comment 23

[Karl Tomlinson (:karlt)]

The testcase in bug 484458 depends on the position of the pointer (and a modal dialog), so an automated test would either need to be based on some other scenario, which we do not yet know, or perhaps we could put the browser into fullscreen view, which would be likely to catch the pointer on systems with a single monitor.

Attachment 362840 [details] [diff] and attachment 363049 [details] [diff] [review] are tested on 1.9.1 and 1.9.2 through their use in code from bug 298889.  (Mochitests would be crashing without these patches.)

1.9.0 doesn't have the code from bug 298889, so we don't have test coverage through that code, and we don't know of another testcase.  We might be able to construct something with different cursor styles, but that would depend on the position of the pointer again.

I suspect the patches here also fix bug 467744, as I haven't seen any "unexpectedly destroyed" messages since applying these patches, but I don't fully understand what's happening in that bug (and it doesn't have a reproducible testcase.

Comment 24

[Samuel Sidler (old account; do not CC)]

Comment on attachment 370612 [details] [diff] [review]
collective patch for 1.9.0

Karl, thanks for the explanation. This patch will have to do then.

Approved for 1.9.0.10. a=ss for release-drivers

Comment 25

[Karl Tomlinson (:karlt)]

Fixed for 1.9.0.10:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1240197420&maxdate=1240197421&who=karlt%2B%25karlt.net

Comment 26

[Martin Stránský [:stransky] (ni? me)]

It doesn't affect 1.8, right?

Comment 27

[Karl Tomlinson (:karlt)]

I don't know the changes between 1.8 and 1.9 well enough to know why the testcases in bug 484458 and bug 474303 do not crash 1.8.  The widget structures are similar to 1.8, but perhaps the order of operations is different enough that no problems are hit.

Comment 28

[Al Billings [:abillings - ex-MoCo]]

I've verified bug 484458 but I cannot verify bug 474303 (the testcase isn't crashing). I'm not sure what else can be done to verify this fix at this time.

Comment 29

[Karl Tomlinson (:karlt)]

I expect this change to GDK will also prevent this bug from happening:
http://svn.gnome.org/viewvc/gtk%2B?view=revision&revision=22420

Comment 30

[Karl Tomlinson (:karlt)]

(In reply to comment #29)
> I expect this change to GDK will also prevent this bug from happening:
> http://svn.gnome.org/viewvc/gtk%2B?view=revision&revision=22420

I mean it would avoid bug 484458 and bug 474303.
It wouldn't fix all of potential problems fixed in this bug.