GNOME Bugzilla – Bug 348588
Hang on "Invalid share name" dialog
Last modified: 2018-05-24 11:40:34 UTC
Rhythmbox hangs when I hit OK in the DAAP "Invalid share name" (DAAP name collision) dialog. Trace: (gdb) t a a bt
+ Trace 69584
MDNS impl is avahi 0.6.11.
Hm. The hang isn't happening in rb code. Possibly an avahi bug; I'll see if 0.6.12 fixes it.
Nah, just need to wrap the dialog in gdk_threads_enter/leave().
Created attachment 69545 [details] [review] daap-collision-hang.patch
Created attachment 69590 [details] [review] patch That should be moved up the stack, because whatever is calling that function should already hold it. I've gone though all the g_idle_add* and g_timeout_add* functions to check that we've taking the GDK lock when needed. This patch (hopefully) fixes everything except backends/gstreamer/rb-encoder-gst.c, rhythmdb/rhythmdb.c and player/rb-recorder-gst.c (which I haven't done yet). We really need to either a) make sure all signals get emitted with it held, or b) document which ones don't.
Patch looks correct to me. I don't think we should hold the gdk lock for all signal emission. A reasonable distinction might be that all backend objects (things with no UI of their own - player, encoder, mdns stuff, metadata, maybe daap connection) emit signals without the lock held, but everything else holds the lock.
I've committed the patch to cvs. I agree about not holding the lock for "backend" objects, but it gets a bit complicated. Unless the object is emitting it from a idle/timeout, it might be holding the lock (depending on whether the calling code was). We'll need to document whether it's being emitted without the lock being held or if it doesn't know. In the latter case, the signal handler will need do whatever it wants by registering a idle callback and taking the lock there, since the lock may not be recursive.
-- 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/rhythmbox/issues/209.