GNOME Bugzilla – Bug 777661
Signals not disconnected in clock manager, causes crashes in gnome-shell
Last modified: 2017-04-06 17:40:20 UTC
This isssue is happening on Fedora 25. % rpm -q gnome-desktop3 gnome-desktop3-3.22.2-2.0.0.1.fif25.x86_64 gnome-desktop3-3.22.2-2.0.0.1.fif25.i686 I've been noticing gnome-shell crashes (see #777569) and has traced it with possible races in gnome-desktop. Please look at the attached patch. From the backtrace in #777569, it looks like the signals for update the clocks via dconf or a timezone update are not disconnected correctly and are firing after the clock object has been recycled. The patch disconnects the signals in the object destructor, and it fixes the gnome-shell crashes for me. This seems to be related to #736775 as well.
Created attachment 344067 [details] [review] Disconnect signals in clock destructor.
Review of attachment 344067 [details] [review]: Can you provide a git formatted patch with your name, a subject and a message explaining the change? ::: gnome-desktop-3.22.2.orig/libgnome-desktop/gnome-wall-clock.c @@ +108,3 @@ + g_signal_handlers_disconnect_by_func (self->priv->tz_monitor, + G_CALLBACK (on_tz_changed), + self); I'd prefer that you changed the signal connection to use g_signal_connect_object() instead. Another way to fix the crash, is to move all of _dispoase() to _finalize() instead. There's actually no reason to use _dispose() in this class since everything we're free'ing here is private so there can't be reference loops. @@ +116,3 @@ + g_signal_handlers_disconnect_by_func (self->priv->desktop_settings, + G_CALLBACK (on_schema_change), + self); same
*** Bug 777569 has been marked as a duplicate of this bug. ***
(In reply to Rui Matos from comment #2) > Review of attachment 344067 [details] [review] [review]: > > Can you provide a git formatted patch with your name, a subject and a > message explaining the change? While I'm happy to reformat the patch to follow the git convention, or perform the changes you've described (move the clean-up code from dispose to finalize), I don't know glib that well, and I think the maintainer is better suited to make such a change. On the other hand, if this is the only way to get a fix committed quickly (the tubes are full of references to this crash), I'll take a stab at it.
I'll close this as a duplicate of bug 780861 which has a better patch. And Rui, you can then explain to me why the signal isn't automatically disconnected when the GSettings is destroyed. Because I couldn't figure it out. *** This bug has been marked as a duplicate of bug 780861 ***