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 693995 - GkColorChooserWidget crashes in select_swatch if palettes are replaced with add_palette
GkColorChooserWidget crashes in select_swatch if palettes are replaced with a...
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
3.7.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2013-02-16 22:50 UTC by Michael Hofmann
Modified: 2013-03-04 23:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Minimal testcase (556 bytes, text/x-csrc)
2013-02-17 14:14 UTC, Michael Hofmann
  Details
Deselect any swatch before removing palettes (582 bytes, patch)
2013-02-17 14:29 UTC, Michael Hofmann
none Details | Review
Deselect any swatch before removing palettes (667 bytes, patch)
2013-02-17 14:51 UTC, Michael Hofmann
none Details | Review
GtkColorChooserWidget: deselect swatch before removing palettes (1.01 KB, patch)
2013-02-17 15:09 UTC, Michael Hofmann
committed Details | Review

Description Michael Hofmann 2013-02-16 22:50:29 UTC
When custom palettes are set with add_palette, the default palettes are removed. If the selected color was on one of those, priv->current does not get unset, resulting in a dangling pointer that will cause a crash in select_swatch when gtk_widget_unset_state_flags is called to deselect the previous swatch.

This seems to make it impossible to use custom palettes, as the widget tries to restore the state from dconf. If the restored color is on one of the default palettes, setting another palette on startup will already cause a crash.

I think this bug is still present in HEAD. 

I don't have a minimal example that causes the bug, but could create one if necessary.
Comment 1 Michael Hofmann 2013-02-17 11:30:27 UTC
Reproduced with jhbuild for git HEAD.
Comment 2 Michael Hofmann 2013-02-17 11:32:35 UTC
(gdb) bt
  • #0 g_type_check_instance_cast
    at gtype.c line 4028
  • #1 select_swatch
    at gtkcolorchooserwidget.c line 103
  • #2 g_closure_invoke
    at gclosure.c line 777
  • #1 select_swatch
    at gtkcolorchooserwidget.c line 103
98	
99	  if (cc->priv->current == swatch)
100	    return;
101	
102	  if (cc->priv->current != NULL)
103	    gtk_widget_unset_state_flags (GTK_WIDGET (cc->priv->current), GTK_STATE_FLAG_SELECTED);
104	  gtk_widget_set_state_flags (GTK_WIDGET (swatch), GTK_STATE_FLAG_SELECTED, FALSE);
105	  cc->priv->current = swatch;
Comment 3 Michael Hofmann 2013-02-17 14:14:24 UTC
Created attachment 236449 [details]
Minimal testcase

Minimal testcase, run with:
  gcc colorchooserpalette.c -o colorchooserpalette -g -O0 $(pkg-config --cflags --libs gtk+-3.0) && G_SLICE=always-malloc G_DEBUG=gc-friendly valgrind ./colorchooserpalette
Comment 4 Michael Hofmann 2013-02-17 14:15:43 UTC
Valgrind output:

==9711== Invalid read of size 8
==9711==    at 0x7E60D90: g_type_check_instance_cast (gtype.c:4022)
==9711==    by 0x4F27D9A: select_swatch (gtkcolorchooserwidget.c:103)
==9711==    by 0x4F28A6B: gtk_color_chooser_widget_set_rgba (gtkcolorchooserwidget.c:761)
==9711==    by 0x400816: main (colorchooserpalette.c:15)
==9711==  Address 0xbd7a890 is 0 bytes inside a block of size 304 free'd
==9711==    at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9711==    by 0x7E5F96A: g_type_free_instance (gtype.c:1957)
==9711==    by 0x4FAA56F: gtk_grid_forall (gtkgrid.c:502)
==9711==    by 0x4F3FCDB: gtk_container_destroy (gtkcontainer.c:1377)
==9711==    by 0x7E3E081: g_closure_invoke (gclosure.c:777)
==9711==    by 0x7E50024: signal_emit_unlocked_R (gsignal.c:3682)
==9711==    by 0x7E58A6F: g_signal_emit_valist (gsignal.c:3314)
==9711==    by 0x7E58CB1: g_signal_emit (gsignal.c:3370)
==9711==    by 0x513A157: gtk_widget_dispose (gtkwidget.c:10743)
==9711==    by 0x7E4336C: g_object_unref (gobject.c:2987)
==9711==    by 0x7E41840: g_cclosure_marshal_VOID__OBJECTv (gmarshal.c:1316)
==9711==    by 0x7E3E2A9: _g_closure_invoke_va (gclosure.c:840)
Comment 5 Michael Hofmann 2013-02-17 14:29:24 UTC
Created attachment 236450 [details] [review]
Deselect any swatch before removing palettes

In remove_palette(...), deselects any selected swatches.

But this will also deselect swatches in the custom palette...
Comment 6 Michael Hofmann 2013-02-17 14:51:37 UTC
Created attachment 236453 [details] [review]
Deselect any swatch before removing palettes

In remove_palette(...), deselects any selected swatch, unless it is on the custom palette
Comment 7 Michael Hofmann 2013-02-17 15:09:45 UTC
Created attachment 236455 [details] [review]
GtkColorChooserWidget: deselect swatch before removing palettes

When adding custom palettes, set the current swatch to NULL if the
palette it is on is going to be removed.
Comment 8 Matthias Clasen 2013-03-04 23:31:39 UTC
Attachment 236455 [details] pushed as 670e532 - GtkColorChooserWidget: deselect swatch before removing palettes