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 729496 - Crash with latest gtk+ from master in gtk_combo_box_get_preferred_width
Crash with latest gtk+ from master in gtk_combo_box_get_preferred_width
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkComboBox
3.13.x
Other Linux
: Normal critical
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2014-05-04 10:26 UTC by Vadim Rutkovsky
Modified: 2014-05-05 20:17 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Vadim Rutkovsky 2014-05-04 10:26:18 UTC
Evo from master crashes when initial setup wizard is about to get displayed:

Program terminated with signal 11, Segmentation fault.

Thread 1 (Thread 0x7fb62d796980 (LWP 1053))

  • #0 gtk_combo_box_get_preferred_width
    at ../../gtk/gtkcombobox.c line 5594
  • #1 port_entry_get_preferred_width
    at ../../e-util/e-port-entry.c line 261
  • #2 gtk_widget_query_size_for_orientation
    at ../../gtk/gtksizerequest.c line 180
  • #3 gtk_widget_compute_size_for_orientation
    at ../../gtk/gtksizerequest.c line 390
  • #4 gtk_widget_get_preferred_width
    at ../../gtk/gtksizerequest.c line 489
  • #5 compute_request_for_child
    at ../../gtk/gtkgrid.c line 690
  • #6 gtk_grid_request_non_spanning
    at ../../gtk/gtkgrid.c line 732
  • #7 gtk_grid_request_run
    at ../../gtk/gtkgrid.c line 1140
  • #8 gtk_grid_get_size
    at ../../gtk/gtkgrid.c line 1446
  • #9 gtk_widget_query_size_for_orientation
    at ../../gtk/gtksizerequest.c line 180
  • #10 gtk_widget_compute_size_for_orientation
    at ../../gtk/gtksizerequest.c line 390
  • #11 gtk_widget_get_preferred_width
    at ../../gtk/gtksizerequest.c line 489
  • #12 gtk_box_get_size
    at ../../gtk/gtkbox.c line 1536
  • #13 gtk_widget_query_size_for_orientation
    at ../../gtk/gtksizerequest.c line 180
  • #14 gtk_widget_compute_size_for_orientation
    at ../../gtk/gtksizerequest.c line 390
  • #15 gtk_widget_get_preferred_width
    at ../../gtk/gtksizerequest.c line 489
  • #16 _gtk_widget_get_preferred_size_for_size
    at ../../gtk/gtksizerequest.c line 868
  • #17 gtk_notebook_size_request
    at ../../gtk/gtknotebook.c line 2283
  • #18 gtk_widget_query_size_for_orientation
    at ../../gtk/gtksizerequest.c line 180
  • #19 gtk_widget_compute_size_for_orientation
    at ../../gtk/gtksizerequest.c line 390
  • #20 gtk_widget_get_preferred_width
    at ../../gtk/gtksizerequest.c line 489
  • #21 gtk_box_get_size
    at ../../gtk/gtkbox.c line 1536
  • #22 gtk_widget_query_size_for_orientation
    at ../../gtk/gtksizerequest.c line 180
  • #23 gtk_widget_compute_size_for_orientation
    at ../../gtk/gtksizerequest.c line 390
  • #24 gtk_widget_get_preferred_width
    at ../../gtk/gtksizerequest.c line 489
  • #25 _gtk_widget_get_preferred_size_for_size
    at ../../gtk/gtksizerequest.c line 868
  • #26 gtk_notebook_size_request
    at ../../gtk/gtknotebook.c line 2283
  • #27 gtk_widget_query_size_for_orientation
    at ../../gtk/gtksizerequest.c line 180
  • #28 gtk_widget_compute_size_for_orientation
    at ../../gtk/gtksizerequest.c line 390
  • #29 gtk_widget_get_preferred_width
    at ../../gtk/gtksizerequest.c line 489
  • #30 gtk_box_get_size
    at ../../gtk/gtkbox.c line 1536
  • #31 gtk_widget_query_size_for_orientation
    at ../../gtk/gtksizerequest.c line 180
  • #32 gtk_widget_compute_size_for_orientation
    at ../../gtk/gtksizerequest.c line 390
  • #33 gtk_widget_get_preferred_width
    at ../../gtk/gtksizerequest.c line 489
  • #34 gtk_box_get_size
    at ../../gtk/gtkbox.c line 1536
  • #35 gtk_widget_query_size_for_orientation
    at ../../gtk/gtksizerequest.c line 180
  • #36 gtk_widget_compute_size_for_orientation
    at ../../gtk/gtksizerequest.c line 390
  • #37 gtk_widget_get_preferred_width
    at ../../gtk/gtksizerequest.c line 489
  • #38 gtk_window_get_preferred_width
    at ../../gtk/gtkwindow.c line 8369
  • #39 gtk_widget_query_size_for_orientation
    at ../../gtk/gtksizerequest.c line 180
  • #40 gtk_widget_compute_size_for_orientation
    at ../../gtk/gtksizerequest.c line 390
  • #41 gtk_widget_get_preferred_width
    at ../../gtk/gtksizerequest.c line 489
  • #42 _gtk_widget_get_preferred_size_and_baseline
    at ../../gtk/gtksizerequest.c line 682
  • #43 gtk_widget_get_preferred_size
    at ../../gtk/gtksizerequest.c line 747
  • #44 gtk_window_compute_hints
    at ../../gtk/gtkwindow.c line 9740
  • #45 gtk_window_compute_configure_request
    at ../../gtk/gtkwindow.c line 9094
  • #46 gtk_window_show
    at ../../gtk/gtkwindow.c line 5468
  • #47 g_closure_invoke
    at ../../gobject/gclosure.c line 768
  • #48 signal_emit_unlocked_R
    at ../../gobject/gsignal.c line 3481
  • #49 g_signal_emit_valist
    at ../../gobject/gsignal.c line 3307
  • #50 g_signal_emit
    at ../../gobject/gsignal.c line 3363
  • #51 gtk_widget_show
    at ../../gtk/gtkwidget.c line 4401
  • #52 gtk_widget_show
    at ../../gtk/gtkwidget.c line 4378
  • #53 startup_wizard_run
    at ../../../modules/startup-wizard/evolution-startup-wizard.c line 182
  • #54 startup_wizard_load_accounts
    at ../../../modules/startup-wizard/evolution-startup-wizard.c line 273
  • #55 g_closure_invoke
    at ../../gobject/gclosure.c line 768
  • #56 signal_emit_unlocked_R
    at ../../gobject/gsignal.c line 3551
  • #57 g_signal_emit_valist
    at ../../gobject/gsignal.c line 3307
  • #58 g_signal_emit
    at ../../gobject/gsignal.c line 3363
  • #59 e_shell_event
    at ../../shell/e-shell.c line 1772
  • #60 main
    at ../../shell/main.c line 677


Probably the crash happens after gtk+ commit: https://git.gnome.org/browse/gtk+/commit/?id=7f60cab47d9651ed3ed53b86f1f74de71b55eee0
Comment 1 Milan Crha 2014-05-05 13:19:33 UTC
This [1] change caused the crash, thus I move this to gtk+. Basically, even if the public API adds always non-NULL parameters, then the descendants doesn't have any good way to call parent class' method through this public API, thus they end with passing whatever they are interested in. It'll be better to revert the change [1].

By the way, evolution's code calls:
	/* Preferred width of a standard combobox */
	GTK_WIDGET_CLASS (e_port_entry_parent_class)->
		get_preferred_width (widget, &parent_width_min, NULL);

[1] https://git.gnome.org/browse/gtk+/commit/gtk/gtkcombobox.c?id=7455ab72f8681cff908526985f8e4f7ff924aec6
Comment 2 Benjamin Otte (Company) 2014-05-05 13:41:53 UTC
I think the correct way is to pass a dummy variable if you don't care about the result. I'd be happy if you changed that in evolution.

I'll add a NULL check in GTK, too, to not make older evolution versions crash.
Comment 3 Milan Crha 2014-05-05 17:07:12 UTC
(In reply to comment #2)
> I think the correct way is to pass a dummy variable if you don't care about the
> result. I'd be happy if you changed that in evolution.

Right, I know it's possible, but the devel doc for gtk_widget_get_preferred_width (and all those other "get_preferred" public API) says it can use NULL for whichever parameter. Of course, it's not the same as calling parent class method, but from my point of view the parent class methods, or basically each implementation of these methods, should follow the devel doc.

Changing this in evolution will not fix all 2^10+ applications using gtk in a similar way as evolution does. Thus I do not see any gain in changing evolution itself (I'm sorry). Another (pseudo) reason why not add a dummy variable is that once the compiler, or any static analyser, gets smart enough and will recognize that the variable is not used, will also claim it on console, while I rely on warnings and try to keep the code clean of them - after all, the compiler/static analyser sometimes knows what it says.
Comment 4 Benjamin Otte (Company) 2014-05-05 20:17:30 UTC
The problem is that we don't ever pass NULL to those vfuncs, so if some implementation doesn't check for NULL it will never get noticed until evolution (or the other 2^10 applications) decide to chain up. And we have enough of those widgets that don't handle the NULL case.

So I'd like to enforce this as an official stance and say that vfuncs can expect non-NULL arguments for the get_preferred_* functions.
After all, everybody passing NULL will notice quickly when their app crashes.