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 724969 - Protect against hashtable destruction during it's being operated
Protect against hashtable destruction during it's being operated
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2014-02-22 20:15 UTC by Carlos Garnacho
Modified: 2014-02-23 01:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
core: prevent early MetaIdleMonitor destruction when its invoker vanishes (1.41 KB, patch)
2014-02-22 20:16 UTC, Carlos Garnacho
reviewed Details | Review
core: prevent early MetaIdleMonitor destruction when its invoker vanishes (1.26 KB, patch)
2014-02-23 00:50 UTC, Carlos Garnacho
committed Details | Review

Description Carlos Garnacho 2014-02-22 20:15:38 UTC
Quite unlikely sequence of events, if you:

1) Have gnome-shell running
2) switch to a tty
3) switch back to the graphical session
4) restart g-s-d through $prefix/libexec/gnome-settings-daemon -r in a terminal

Then you either get crashes or infinite loops when the DBus name a MetaIdleMonitor was looking on vanishes (presumably g-s-d's), it turns out the MetaIdleMonitor is destroyed at the time monitor->watches is being removed that watch that happened to keep the last reference to the monitor.

From my investigation this happens due to the tty switch, that will make Xorg remove, and then add back the devices, however at the time of removing a few of these objects are kept alive by the caller, which keeps a reference. When the dbus name vanishes, the watch is removed from the hashtable, and the GDestroyNotify called on its data, which destroys the MetaIdleMonitor within g_hash_table_remove(), as the hashtable itself is destroyed, bad things happen after.

I'm attaching a patch that wraps that in a ref/unref pair, so the hashtable is destroyed at a better time in that case.
Comment 1 Carlos Garnacho 2014-02-22 20:16:18 UTC
Created attachment 270010 [details] [review]
core: prevent early MetaIdleMonitor destruction when its invoker vanishes

If the last reference of a MetaIdleMonitor is held by the caller, it may
happen that the last reference is lost when calling the GDestroyNotify,
if this happens when the watched DBus name vanishes, the object (and the
watches hashtable) are destroyed while manipulating the watches hashtable,
so bad things may happen then.

Fix this by wrapping the operation by a ref/unref pair, so the object would
be destroyed after operating on the hashtable.
Comment 2 Jasper St. Pierre (not reading bugmail) 2014-02-22 20:57:51 UTC
Review of attachment 270010 [details] [review]:

Could you move the ref/unref to remove_watch?
Comment 3 Carlos Garnacho 2014-02-23 00:50:55 UTC
Created attachment 270024 [details] [review]
core: prevent early MetaIdleMonitor destruction when its invoker vanishes

If the last reference of a MetaIdleMonitor is held by the caller, it may
happen that the last reference is lost when calling the GDestroyNotify,
if this happens when the watched DBus name vanishes, the object (and the
watches hashtable) are destroyed while manipulating the watches hashtable,
so bad things may happen then.

Fix this by wrapping the operation by a ref/unref pair, so the object would
be destroyed after operating on the hashtable.
Comment 4 Jasper St. Pierre (not reading bugmail) 2014-02-23 01:03:47 UTC
Review of attachment 270024 [details] [review]:

Thanks.
Comment 5 Carlos Garnacho 2014-02-23 01:08:58 UTC
Attachment 270024 [details] pushed as 1e01a55 - core: prevent early MetaIdleMonitor destruction when its invoker vanishes