Created attachment 381947 [details] PATCH: fix gvfsdeamon backend cleanup function not being called on unmount gvfs <= 1.5.1 does not properly call the finalize function of backends, due to a missing unref call. This causes the cleanup functions of the libraries underlying the backends to not get called. In case of the gphoto2 backend, this causes the kernel driver for dual mode webcams (which have a kernel space webcam driver and a userspace stillcam driver), to not get re-attached to the device when then the gvfs mount gets unmounted. This patch fixes this by adding a g_object_unref (job) to g_vfs_daemon_initiate_mount, which is needed as g_vfs_daemon_queue_job takes a reference itself. Note this patch applies to F-12 too, and it would be nice to get it in there too, as this breaks the use of a lot of different dual mode webcam types as webcam.
Also filed upstream: https://bugzilla.gnome.org/show_bug.cgi?id=606194
Some explanation about the patch, to hopefully speed up reviewing it. David I know you said you don't have any spare cycles for this, but please make a few it is a really trivial patch, and this bug blocks fixing bug 552890, which makes still cams not working out of the box with F-12. About the patch: g_vfs_daemon_initiate_mount() creates a new g_vfs_job_mount, storing the returned object reference in a local variable, then it queues it by calling g_vfs_daemon_queue_job(), which will takes it own reference to it: void g_vfs_daemon_queue_job (GVfsDaemon *daemon, GVfsJob *job) { g_debug ("Queued new job %p (%s)\n", job, g_type_name_from_instance ((gpointe g_object_ref (job); g_signal_connect (job, "finished", (GCallback)job_finished_callback, daemon); g_signal_connect (job, "new_source", (GCallback)job_new_source_callback, daem ... And then g_vfs_daemon_initiate_mount() ends, without unreferencing the job it created, so now the refcount of the job is 2, even though only the job queue is holding a reference (the local variable reference lifetime ends with leaving g_vfs_daemon_initiate_mount()). My patch fixes this by correctly unreferencing the job when g_vfs_daemon_initiate_mount() ends, as with that ending its reference to the job is no more: --- gvfs-1.5.1/daemon/gvfsdaemon.c~ 2009-11-18 13:14:51.000000000 +0100 +++ gvfs-1.5.1/daemon/gvfsdaemon.c 2010-01-06 11:19:38.000000000 +0100 @@ -1081,6 +1081,7 @@ g_vfs_daemon_initiate_mount (GVfsDaemon job = g_vfs_job_mount_new (mount_spec, mount_source, is_automount, request, g_vfs_daemon_queue_job (daemon, job); + g_object_unref (job); } /**
(In reply to comment #2) > Some explanation about the patch, to hopefully speed up reviewing it. David I > know you said you don't have any spare cycles for this, but please make a few > it is a really trivial patch, and this bug blocks fixing bug 552890, which > makes still cams not working out of the box with F-12. Sure. Actually, Alexander Larsson is the GVfs maintainer and wrote the code in question (which is not gphoto2 specific). So I'll ask him to review it - he's back on Monday and is in your timezone - FWIW, he's alexl on IRC and hangs out in #fedora-desktop. > About the patch: Thanks for the explanation - it will make it easier to review. In the future, if you could use the upstream bug for such things, things will go faster and smooth as a lot more people with GVfs experience reads the upstream bugzilla. Thanks!
David, thanks for pointing me to Alex, Tomáš, Alex has reviewed the patch and committed it upstream, could you do a F-12 update with this patch and the patch from bug 552856 please (or give me an ok to do it myself). Then I can continue with fixing bug 552890, which this bug is blocking. Thanks, Hans
(In reply to comment #4) > Tomáš, > > Alex has reviewed the patch and committed it upstream, could you do a F-12 > update > with this patch and the patch from bug 552856 please (or give me an ok to do it > myself). Then I can continue with fixing bug 552890, which this bug is > blocking. > > Thanks, > > Hans Sure, I'll do that.
gvfs-1.4.3-2.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.