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 708289 - GNOME Shell use-after-free crash on wakeup
GNOME Shell use-after-free crash on wakeup
Status: RESOLVED OBSOLETE
Product: gnome-desktop
Classification: Core
Component: libgnome-desktop
1.5.x
Other Linux
: Normal normal
: ---
Assigned To: Desktop Maintainers
Desktop Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-09-18 13:40 UTC by David Woodhouse
Modified: 2018-09-21 16:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
idle-monitor: hold idle monitor ref across all alarm callbacks instad of each individually (3.28 KB, patch)
2013-09-18 14:18 UTC, Ray Strode [halfline]
none Details | Review

Description David Woodhouse 2013-09-18 13:40:11 UTC
On waking from DPMS, I am seeing frequent shell crashes:

Thread no. 1 (10 frames)
 #0 g_object_ref at gobject.c:2884
 #1 fire_watch at gnome-idle-monitor.c:148
 #2 g_list_foreach at glist.c:949
 #3 handle_alarm_notify_event at gnome-idle-monitor.c:193
 #4 xevent_filter at gnome-idle-monitor.c:215
 #5 gdk_event_apply_filters at gdkeventsource.c:81
 #6 gdk_event_source_translate_event at gdkeventsource.c:195
 #7 _gdk_x11_display_queue_events at gdkeventsource.c:338
 #8 gdk_display_get_event at gdkdisplay.c:313
 #14 meta_run at core/main.c:556

See the downstream bug https://bugzilla.redhat.com/show_bug.cgi?id=982766 for full details, and valgrind output.
Comment 1 Ray Strode [halfline] 2013-09-18 14:18:25 UTC
Created attachment 255221 [details] [review]
idle-monitor: hold idle monitor ref across all alarm callbacks instad of each individually

If one callback unrefs the monitor, we need to make sure it stays alive
for the other callbacks in flight.

This commit moves the ref/unref pair up a level to prevent the monitor
from getting freed prematurely.
Comment 2 Ray Strode [halfline] 2013-09-18 14:19:01 UTC
the above patch has not be tested, but seems plausible given the backtrace.
Comment 3 David Woodhouse 2013-09-18 15:42:58 UTC
Thanks. I'm now running it on my laptop where I've been seeing the crash on about one in three wakeups, so if it does fix the problem then I'll fairly soon have some confidence in it.
Comment 4 Giovanni Campagna 2013-09-20 12:53:06 UTC
(In reply to comment #1)
> Created an attachment (id=255221) [details] [review]
> idle-monitor: hold idle monitor ref across all alarm callbacks instad of each
> individually
> 
> If one callback unrefs the monitor, we need to make sure it stays alive
> for the other callbacks in flight.
> 
> This commit moves the ref/unref pair up a level to prevent the monitor
> from getting freed prematurely.

This patch surely looks correct, but we probably want to keep the monitor alive when freeing the watches too. See the investigation in bug 708420.
Comment 5 David Woodhouse 2013-09-21 21:08:22 UTC
I've just seen another crash using the patch in comment #1.

(gdb) t a a bt full


Comment 6 Giovanni Campagna 2013-09-22 17:52:46 UTC
Right, protecting the monitor is not enough, if we can still free the watches after they are extracted from the table but before they're invoked.
I guess we need reference counting.
Comment 7 David Woodhouse 2013-11-07 11:04:29 UTC
Any chance of an updated patch to test, please? This is still crashing gnome-shell extremely frequently when I wake the machine.
Comment 8 GNOME Infrastructure Team 2018-09-21 16:36:46 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-desktop/issues/52.