GNOME Bugzilla – Bug 720482
old socket dirs are not removed
Last modified: 2014-04-23 07:33:09 UTC
Hi. It seems gvfs does not clean up its socket directory properly under some circumstances. After a few days running GNOME: $ find /tmp -type d -name '*gvfs*' | wc -l 68 They still all contain a socket file like: /tmp/gvfs-username-XXXXXXXX/socket and some dates from when the laptop was started (i.e. few days ago). I am running on OpenBSD, I think this is not seen on Linux because of abstract sockets (I may be wrong though). new_connection_data_free() (daemon/gvfsdaemon.c) has this code: /* Remove the socket and dir after connected */ if (data->socket_dir) rmdir (data->socket_dir); So in case the socket dir is not empty, which is the case here since it contains a socket file, the directory is not removed. Note that I am not sure this is really where it happens... but well, I would like to find out why these directories do not get removed. Thank you.
Yeah, it seems likely that the problem is just that the rmdir fails due to a non-empty dir. Can you try just removing socket_dir/socket before the rmdir? That seems like the correct fix to me.
In general the code works like this: * Each gvfs daemon is connected to the session bus. * Clients connect to the daemon via the session bus, sending a GetConnection request when they want to set up a peer-to-peer connection. * The daemon then creates a unix domain socket and starts up a server on it, returning the address over the session bus. * The client connects to the address * The daemon deletes the socket as there will be no more users connecting to it.
> Can you try just removing socket_dir/socket before the rmdir? That seems like > the correct fix to me. Hi Alexander. This patch does the trick for me.
Created attachment 266889 [details] [review] remove socket before socket_dir
Any comments about it? FYI that patch has been applied to the gvfs package in OpenBSD-current to widen the testing and so far everything works as advertised. Thanks.
Review of attachment 266889 [details] [review]: Thank you for the patch. Just a few notes... ::: daemon/gvfsdaemon.c @@ +607,2 @@ rmdir (data->socket_dir); + if (socket) I think the if condition isn't necessary... @@ +916,3 @@ rmdir (socket_dir); + if (socket) + g_free (socket); Wouldn't be better to use the new_connection_data_free function instead?
(In reply to comment #6) > Review of attachment 266889 [details] [review]: > > Thank you for the patch. Just a few notes... > > ::: daemon/gvfsdaemon.c > @@ +607,2 @@ > rmdir (data->socket_dir); > + if (socket) > > I think the if condition isn't necessary... > > @@ +916,3 @@ > rmdir (socket_dir); > + if (socket) > + g_free (socket); > > Wouldn't be better to use the new_connection_data_free function instead? You are right, that is way better. New patch attached. Thank you for the review :-)
Created attachment 267042 [details] [review] remove socket before socket_dir v2
Review of attachment 267042 [details] [review]: ::: daemon/gvfsdaemon.c @@ +909,1 @@ g_free (data); data is freed in new_connection_data_free already...
Created attachment 267046 [details] [review] remove socket before socket_dir v3 *sigh* not my day... Here you go, sorry about that.
Comment on attachment 267046 [details] [review] remove socket before socket_dir v3 Thanks! commit 2909ef002746201a83922fdd1c5efad6766347f8
gnome-3-10: commit 21239e01037c6375518e3dbbd8548634b6e500ac
*** Bug 547412 has been marked as a duplicate of this bug. ***