After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 727644 - SEGV in gtk_window_propagate_key_event()
SEGV in gtk_window_propagate_key_event()
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
3.12.x
Other All
: Normal major
: ---
Assigned To: gtk-bugs
gtk-bugs
3.12.1
Depends on:
Blocks:
 
 
Reported: 2014-04-05 08:44 UTC by Fredy Paquet
Modified: 2014-04-12 05:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed Patch taken from Gtk 2.24.21 (1.25 KB, patch)
2014-04-05 09:03 UTC, Fredy Paquet
none Details | Review
window: Be more careful when propagating key events (1.06 KB, patch)
2014-04-05 15:45 UTC, Matthias Clasen
committed Details | Review

Description Fredy Paquet 2014-04-05 08:44:50 UTC
Description

We have numerous reports from our customer base reporting SEGV in our Gtk+ application. Taking multiple stack traces always gives us an image similar to this:

Program received signal SIGSEGV, Segmentation fault.
 0x0000000063a49d65 in g_object_ref () from C:\Casymir\bin\libgobject-2.0-0.dll
 (gdb) bt
 0 0x0000000063a49d65 in g_object_ref ()
    from C:\Casymir\bin\libgobject-2.0-0.dll
0000001 0x000000000062e0c0 in ?? ()
 2 0x00000000005ab440 in ?? ()
 3 0x00000000082336b0 in ?? ()
 4 0x000000000062e0c0 in ?? ()
 5 0x000000000062e0c0 in ?? ()
 6 0x0000000061a54e58 in gtk_window_propagate_key_event ()
    from C:\Casymir\bin\libgtk-win32-2.0-0.dll
 7 0x0000000061a54f10 in gtk_window_key_press_event ()
    from C:\Casymir\bin\libgtk-win32-2.0-0.dll
 8 0x00000000618a8030 in _gtk_marshal_BOOLEAN__BOXED ()
    from C:\Casymir\bin\libgtk-win32-2.0-0.dll
 9 0x0000000005e29b60 in ?? ()
 10 0x0000000000000000 in ?? ()
 (gdb)

Tracking down the situation, we found that the loop TERMINATION CONDITION is missing in 
gtk_window_propagate_key_event(). 

The documentation says: Propagate a key press or release event to the focus widget and up 
the focus container chain until a widget handles @event. But in fact, the loop body doesn't 
check the 'handled' return value from gtk_widget_event(), thus always tries to propagate the
pending event up to the toplevel container. 

Analysis:

The point is, that if any event handler destroys part of the widget tree within the call to
gtk_widget_event(), some parent widget may have been destroyed. 

Or in other words: while processing the focus widget, the reference count is correctly incremented 
before the call to gtk_widget_event() and correctly released after. Because not the whole 
parent container chain was ref'd, some of the parent widgets may have been destroyed by the handler.

Context:

- The bug can be reproduced on several operating systems: Linux x86_64, MinGW W32, MinGW W64
- Affected Gtk+ Versions: All, from 2.24.21 up to 3.12.0 (current master branch)
- Code location
  - Gtk+ 2.24.21: gtkwindow.c: function gtk_window_propagate_key_event(): near Line 5179
  - Gtk+ 3.12.0: gtkwindow.c: function gtk_window_propagate_key_event(): near Line 7594
Comment 1 Fredy Paquet 2014-04-05 08:56:29 UTC
Analysis Details:

By putting some debug output into gtk_window_propagate_key_event() and also into the widget destruction handler, you can see below that:

1. the widget 0xe86a60 GtkItemEntry was destroyed at line 7 of the output below
2. gtk_widget_event() returns handled == 1
3. gtk_window_propagate_key_event() continues to walk up the parent tree
4. GLib emits critical warnings for invalid parent widgets


** (opg:23442): DEBUG: KredZahlValHilite: CurField:FAKTZADatum_G
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xc55070 window1
** (opg:23442): DEBUG: KredZahlValLowlite: CurField:FAKTZADatum_G
** (opg:23442): DEBUG: KredZahlValPost: CurField:FAKTZADatum_G
** (opg:23442): DEBUG: KredZahlValLowlite: CurField:FAKTZADatum_G
** (opg:23442): DEBUG: gtk_item_entry_dispose 0xe86a60
** (opg:23442): DEBUG: gtk_item_entry_destroy 0xe86a60 GtkItemEntry
** (opg:23442): DEBUG: KredZLBearFldPost: CurField:FAKTZADatum
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 GtkMessageDialog
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 focus 0x160f8f0 isw 1 handled 0 GtkButton 
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 parent 0x1601af0 isw 1 iso 1 GtkHButtonBox
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 focus 0x1601af0 isw 1 handled 0 GtkHButtonBox 
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 parent 0x15b2db0 isw 1 iso 1 GtkVBox
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 focus 0x15b2db0 isw 1 handled 0 GtkVBox 
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 parent 0xba64a0 isw 1 iso 1 GtkMessageDialog
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 GtkMessageDialog
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 focus 0x160f8f0 isw 1 handled 0 GtkButton 
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 parent 0x1601af0 isw 1 iso 1 GtkHButtonBox
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 focus 0x1601af0 isw 1 handled 0 GtkHButtonBox 
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 parent 0x15b2db0 isw 1 iso 1 GtkVBox
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 focus 0x15b2db0 isw 1 handled 0 GtkVBox 
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xba64a0 parent 0xba64a0 isw 1 iso 1 GtkMessageDialog
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xc55070 focus 0xe86a60 isw 1 handled 1 GtkItemEntry 

Breakpoint 1, g_log (log_domain=0x7ffff7757cf7 "Gtk", 
    log_level=G_LOG_LEVEL_CRITICAL, 
    format=0x7ffff56f3131 "%s: assertion '%s' failed") at gmessages.c:1021
1021	{
(gdb) bt
  • #0 g_log
    at gmessages.c line 1021
  • #1 IA__gtk_widget_get_name
    at gtkwidget.c line 5841
  • #2 IA__gtk_window_propagate_key_event
    at gtkwindow.c line 5217
  • #3 gtk_window_key_press_event
    at gtkwindow.c line 5247
  • #4 _gtk_marshal_BOOLEAN__BOXED
    at gtkmarshalers.c line 86
  • #5 g_closure_invoke
    at gclosure.c line 777
  • #6 signal_emit_unlocked_R
    at gsignal.c line 3624
  • #7 g_signal_emit_valist
    at gsignal.c line 3340
  • #8 g_signal_emit
    at gsignal.c line 3386
  • #9 gtk_widget_event_internal
    at gtkwidget.c line 5010
  • #10 IA__gtk_propagate_event
    at gtkmain.c line 2464
  • #11 IA__gtk_main_do_event
    at gtkmain.c line 1685
  • #12 gdk_event_dispatch
    at gdkevents-x11.c line 2403
  • #13 g_main_dispatch
    at gmain.c line 3065
  • #14 g_main_context_dispatch
    at gmain.c line 3641
  • #15 g_main_context_iterate
    at gmain.c line 3712
  • #16 g_main_loop_run
    at gmain.c line 3906
  • #17 IA__gtk_main
    at gtkmain.c line 1257
  • #18 opg_run_ui
  • #19 main
Continuing.

(opg:23442): Gtk-CRITICAL **: IA__gtk_widget_get_name: assertion 'GTK_IS_WIDGET (widget)' failed
(opg:23442): Gtk-DEBUG: gtk_window_propagate_key_event: 0xc55070 parent 0xe85840 isw 0 iso 0 (null)

Breakpoint 1, g_log (log_domain=0x7ffff5995864 "GLib-GObject", 
    log_level=G_LOG_LEVEL_CRITICAL, 
    format=0x7ffff56f3131 "%s: assertion '%s' failed") at gmessages.c:1021
1021	{
(gdb) 


This output is based on the following debug code:

gboolean
gtk_window_propagate_key_event (GtkWindow        *window,
                                GdkEventKey      *event)
{
  gboolean handled = FALSE;
  GtkWidget *widget, *focus;

  g_return_val_if_fail (GTK_IS_WINDOW (window), FALSE);

  widget = GTK_WIDGET (window);
  focus = window->focus_widget;
g_debug("gtk_window_propagate_key_event: %p %s", widget, gtk_widget_get_name(widget));
  if (!G_IS_OBJECT(focus)) return FALSE;  /* 9.4.13/fp */

  if (focus)
    g_object_ref (focus);
  
  while (!handled &&
         focus && focus != widget &&
         gtk_widget_get_toplevel (focus) == widget)
    {
      GtkWidget *parent;
      
      if (gtk_widget_is_sensitive (focus))
        handled = gtk_widget_event (focus, (GdkEvent*) event);

#if 0
      if (handled)
      {
	  if (G_IS_OBJECT(focus)) g_object_unref(focus);  /* 05.04.14/fp */
	  return TRUE;
      }
#endif
      g_debug("gtk_window_propagate_key_event: %p focus %p isw %d handled %d %s ", widget, focus, G_IS_OBJECT(focus), handled, gtk_widget_get_name(focus)); 

      if (!G_IS_OBJECT(focus)) return handled;  /* 9.4.13/fp */

      parent = focus->parent;

g_debug("gtk_window_propagate_key_event: %p parent %p isw %d iso %d %s", widget, parent, G_IS_OBJECT(parent), G_IS_OBJECT(parent), gtk_widget_get_name(parent));


      if (parent)
        g_object_ref (parent);
      
      g_object_unref (focus);
      
      focus = parent;
    }
  
  if (focus)
    g_object_unref (focus);

  return handled;
}
Comment 2 Fredy Paquet 2014-04-05 09:03:55 UTC
Created attachment 273612 [details] [review]
Proposed Patch taken from Gtk 2.24.21

The patch works by adding

- two checks for existance of the focus objects
- a loop termination condition with reference count cleanup

By adding the patch, all critical warnings are gone.
Comment 3 Matthias Clasen 2014-04-05 15:45:13 UTC
Created attachment 273630 [details] [review]
window: Be more careful when propagating key events

We are keeping references on the widget we are handling as we
are iterating up, but that doesn't protect us against the entire
tree being axed from inside gtk_widget_handle_event.
Comment 4 Matthias Clasen 2014-04-05 15:49:46 UTC
does this patch work for you ?
Comment 5 Fredy Paquet 2014-04-06 07:29:48 UTC
In fact, your patch ist much simpler.
I can confirm it works in our test environment.

---
As far as i understand today, 
the situation /* 9.4.13/fp */ where the focus object destroys itself 
cannot lead to a crash because a focus object reference ist kept.

many thanks

-> FIXED
Comment 6 Matthias Clasen 2014-04-12 05:01:37 UTC
Attachment 273630 [details] pushed as 2d7b927 - window: Be more careful when propagating key events