GNOME Bugzilla – Bug 775074
GtkScrolledWindow does not disconnect all GtkAdjustment signal handlers it connects
Last modified: 2017-08-31 22:34:44 UTC
Created attachment 340743 [details] test case After I wrote in bug 774790 about the thoretical possibility of signal emission with stale user_data, I sketched a test case mimicking a diff viewer with 2 text views scrolling together, after the user closes one. It is attached to this report. Executing it under valgrind and scrolling the mouse wheel over the GtkTextView shows that a GtkScrolledWindow does not disconnect all handlers it connects to adjustment signals. In gtk_scrolled_view_set_{h,v}adjustment (and gtk_scrolled_window_destroy): https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c?h=gtk-3-22#n2280 it disconnects gtk_scrolled_window_adjustment_changed, but few rows below it connected also gtk_scrolled_window_adjustment_value_changed, which is never disconnected. Also indicators connect to adjustments signal that are not disconnected during destruction because at unrealize time indicator_reset is called which set indicator->scrollbar = NULL; https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c?h=gtk-3-22#n4497 and in gtk_scrolled_window_destroy the call to remove_indicator is useless https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c?h=gtk-3-22#n2778 as it does nothing when indicator->scrollbar is NULL; https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c?h=gtk-3-22#n4349
Created attachment 358886 [details] [review] ScrolledWindow: Disconn. Adjustment::value-changed We connected to this when setting a new adjustment, but we never disconnected when changing from that adjustment to a different one.
It's a little less obvious how to handle this one: (In reply to Massimo from comment #0) > Also indicators connect to adjustments signal that are not disconnected > during > destruction because at unrealize time indicator_reset is called which set > indicator->scrollbar = NULL; > > https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c?h=gtk-3- > 22#n4497 > > and in gtk_scrolled_window_destroy the call to remove_indicator is useless > > https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c?h=gtk-3- > 22#n2778 > > as it does nothing when indicator->scrollbar is NULL; > > https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c?h=gtk-3- > 22#n4349 (as the disconnection is probably not the only thing that gets missed out here)
Created attachment 358891 [details] [review] ScrolledWindow: Disconn. Adjustment::value-changed We connected to this when setting a new adjustment, but we never disconnected when changing from that adjustment to a different one. -- missed one
Created attachment 358892 [details] [review] ScrolledWindow: Disconnect indicator_value_changed …in gtk_scrolled_window_destroy(), as remove_indicator() does not.
Created attachment 358893 [details] [review] ScrolledWindow: Disconn. Adjustment::value-changed We connected to this when setting a new adjustment, but we never disconnected when changing from that adjustment to a different one. one more time with less git fubar
Created attachment 358894 [details] [review] ScrolledWindow: Disconnect indicator_value_changed …in gtk_scrolled_window_destroy(), as remove_indicator() does not.
committed in squashed form, using disconnect_by_data() as that's more concise. Thanks!