GNOME Bugzilla – Bug 731077
Race between dbus interface unexport and client methods
Last modified: 2018-09-21 17:42:18 UTC
Created attachment 277684 [details] test program When the Unmount() method is called on a daemon, the daemon unexports the Mount interface and then goes into a shutdown procedure. Any operation which is called on that daemon after this fails with "No such interface 'org.gtk.vfs.Mount' on object at path /org/gtk/vfs/mount/1". Operations can be called on that same daemon due to the GMountInfo cache. Entries are only invalidated when the peer connection closes, which only happens when the daemon actually exits (some time after the Mount interface is unexported). Even without the GMountInfo cache, an operation executed concurrently with unmount() could fail in this way. The only solution I can think of would be to retry operations that fail with G_DBUS_ERROR_UNKNOWN_METHOD after invalidating the GMountInfo cache entry. This would fix the problem, and AFAICT, would not have any side effects or races. I guess the question is whether it is a good solution or not. Experts' comments wanted :-) Attached is a simple example to reproduce the problem. Sample output: """ $ ./unmount mount starting mount finishing mount finished finding enclosing mount finding enclosing mount finished unmount starting unmount finishing unmount finished query_info error: No such interface 'org.gtk.vfs.Mount' on object at path /org/gtk/vfs/mount/1 """
It's possibly even easier to reproduce this by simply writing a program that repeatedly mounts and unmounts the same location, no concurrency needed to reproduce the problem.
*** Bug 733694 has been marked as a duplicate of this bug. ***
bug 733694 comes with a patch... Replying to bug 733694: nice find and analysis, pretty much the same as what I found a few weeks ago. I haven't looked thoroughly through the patch, but as you said, it doesn't solve all races. I think a better approach is (as I mentioned above) to handle unknown method errors in GDaemonFile by expiring the cache entry and retrying. This way even if two threads are completely unsynchronized there shouldn't be a condition where the ugly "No such interface" error is exposed to user programs. I prototyped this for some of the GDaemonFile methods and it worked nicely, but I haven't got round to to making a proper patch yet. Thanks for looking into it... you're welcome to try out the approach of handling G_DBUS_ERROR_UNKNOWN_METHOD :-P
(In reply to comment #1) > It's possibly even easier to reproduce this by simply writing a program that > repeatedly mounts and unmounts the same location, no concurrency needed to > reproduce the problem. That turns out to have been a good conjecture :-) (In reply to comment #0) > The only solution I can think of would be to retry operations that fail with > G_DBUS_ERROR_UNKNOWN_METHOD after invalidating the GMountInfo cache entry. This > would fix the problem, and AFAICT, would not have any side effects or races. I > guess the question is whether it is a good solution or not. I think I prefer my suggestion from the duplicate bug, but yes this seems reasonable. You should also handle the newer UnknownObject and UnknownInterface errors in GLib 2.41.1 if you go this route.
I think Unmount should certainly invalidate the cache, even if nothing else insta-invalidates it, because a successful unmount is exactly the removal of the object. I'm a little concerned that if object paths are ever re-used, we could end up doing something to an object that is not the one we intended. Is the same object path ever recycled within a gvfs backend process?
(In reply to comment #4) > (In reply to comment #0) > > The only solution I can think of would be to retry operations that fail with > > G_DBUS_ERROR_UNKNOWN_METHOD after invalidating the GMountInfo cache entry. This > > would fix the problem, and AFAICT, would not have any side effects or races. I > > guess the question is whether it is a good solution or not. > > I think I prefer my suggestion from the duplicate bug, but yes this seems > reasonable. You should also handle the newer UnknownObject and UnknownInterface > errors in GLib 2.41.1 if you go this route. > > I think Unmount should certainly invalidate the cache, even if nothing else > insta-invalidates it, because a successful unmount is exactly the removal of > the object. Yeah, I actually think both approaches is the best idea. Invalidating the cache when Unmount is called handles most cases but handling UNKNOWN_METHOD (and equivalents) is still needed for a proper, complete solution, so it should be done at some stage. I will take a look at your patch and see if it works for a first step. > > I'm a little concerned that if object paths are ever re-used, we could end up > doing something to an object that is not the one we intended. Is the same > object path ever recycled within a gvfs backend process? Not sure about this one, will need to investigate...
(In reply to comment #5) > I think Unmount should certainly invalidate the cache, even if nothing else > insta-invalidates it, because a successful unmount is exactly the removal of > the object. > Of course, there's no guarantee that Unmount succeeds but there shouldn't be a problem if the cache is invalidated spuriously.
I have committed your patch, but will leave this open since there is still a potential (but much reduced) race condition. Thanks for the work!
(In reply to comment #7) > Of course, there's no guarantee that Unmount succeeds but there shouldn't be a > problem if the cache is invalidated spuriously. Yeah, that was pretty much my thought; that's why my patch does the invalidation immediately, before it checks whether Unmount() even succeeded. (In reply to comment #3) > Thanks for looking into it... you're welcome to try out the approach of > handling G_DBUS_ERROR_UNKNOWN_METHOD :-P Sorry, I don't think I have time right now to trace through all the other o.g.vfs.Mount methods and make sure they store enough context that they can retry.
-- 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/gvfs/issues/235.