GNOME Bugzilla – Bug 684093
Gnome-shell-hotplug-sniffer block access to mounted sshfs filesystem
Last modified: 2012-10-03 17:56:56 UTC
Hi. I'm using Gnome shell 3.4.2 on Debian unstable. I'm experiencing the following bug: I need to mount a filesystem with sshfs (via fuse) in a subdirectory of my home. When I do so, gnome-shell-hotplug-sniffer starts searching through this filesystem to find interesting things. Unfortunately this file is a whole filesystem of another computer, and it includes things like /proc/kmem and similars. What happens is that I can't access the mounted filesystem (every read access blocks forever). I think that gnome-shell-hotplug-sniffer uses so aggressively the filesystem (and sshfs has probably problems with such intense and parallel usage), that fuse isn't able to handle the situation and starts blocking everything. I think that Gnome shell should be somewhat more considerate when scanning a big filesystem, or at least that there should be some way to disable it. This bug was already triaged with Giovanni Campagna. Thanks, Giovanni (yes, that's my name too!).
BTW, the workaround suggested by Giovanni Campagna is to create a directory named /DCIM/100ABCDE in the mounted filesystem. It worked!
My simple solution (doable for 3.6, I think) is to check for autorun-never GSettings key before spawning the hotplug-sniffer or accessing the filesystem in any way.
Created attachment 224413 [details] [review] Autorun: don't scan the filesystem if autorun is disabled Content-type scanning can be expensive, but in some situations it's difficult to know so in advance, in particular for remote filesystems that are exposed as native, such as nfs or fuse-sshfs. Allowing the user to disable autorun entirely in this situation is an acceptable workaround. The only behavioural change with this patch, other than not accessing mounted filesystems, is that the resident notification brings you to Files, rather than the application configured to handle the specific device. I don't think this is a problem, as a user disabling autorun entirely is likely to consider our choice of applications inappropriate anyway.
I thought there was a time limit to these attempts to find a content type? It probably does not make sense to crawl a filesystem for more than a few seconds: after that, the user has opened the filesystem by his own devices anyway.
The limit is 1500 milliseconds. On a decent system, with 10 parallel threads and non-blocking IO, this means a lot of requests, and even if the requestor process id dead, the fuse fs has to answer them.
Can't we exclude sshfs/other types of remote FS from autorun?
Review of attachment 224413 [details] [review]: ::: js/ui/components/autorunManager.js @@ +184,3 @@ + + _processMount: function(mount, hotplug) { + if (!this._settings.get_boolean(SETTING_DISABLE_AUTORUN)) { I hate double negatives. let autorunEnabled = !this._settings.get_boolean(SETTING_DISABLE_AUTORUN); if (autorunEnabled) { ... @@ +198,3 @@ + this._residentSource.addMount(mount, apps); + if (hotplug) + this._transDispatcher.addMount(mount, apps, contentTypes); ... this wasn't tested, was it? (and this code should probably be split out between the two paths)
(In reply to comment #7) > Review of attachment 224413 [details] [review]: > > [...] > > @@ +198,3 @@ > + this._residentSource.addMount(mount, apps); > + if (hotplug) > + this._transDispatcher.addMount(mount, apps, contentTypes); > > ... this wasn't tested, was it? It was tested. I noticed contentTypes was not there (because the other path caused an error), thought it would be irrelevant as addMount() returns if autorun is disable, but obviously forgot the ReferenceError. > (and this code should probably be split out between the two paths) Which two paths? Hotplug vs not hotplug, or autorun vs no autorun? (It used to be split, but I like it more if it's consolidated)
(In reply to comment #6) > Can't we exclude sshfs/other types of remote FS from autorun? I don't know. I could check filesystem::type, but who decides the content of the white/blacklist? Is it only known local fs, like ext2/3/4, btrfs, xfs, hfs... or only known network fs, like nfs, sshfs, anything -fuse... Do we simply wait for users to file another bug if they have an exotic FS that can't handle the hotplug sniffer?
Created attachment 224451 [details] [review] autorunManager: Don't scan the filesystem if autorun is disabled Content-Type scanning can be expensive, and the user disabled autorun, so...
Created attachment 224452 [details] [review] autorunManager: Don't try to scan remote filesystems Content-Type scanning can be super expensive. The autorun manager is meant for local filesystems that are plugged into a USB port or similar, not remote NFS or sshfs mounts. What about something like this?
Review of attachment 224451 [details] [review]: ::: js/ui/components/autorunManager.js @@ +183,3 @@ }, + _contentTypeDiscovered: function(mount, apps, contentTypes, hotplug) { Ehm, where is this hotplug coming from?
Review of attachment 224452 [details] [review]: Your patch is wrong (or at least, it's a behavior change I don't agree with): it hides all network mounts from the transient dispatcher and the resident source, meaning that you can't launch or stop network locations from the shell.
Created attachment 224700 [details] [review] autorunManager: Clean up mount operations Add a processMount function that's shared between mount scanning and hotplug.
Created attachment 224701 [details] [review] autorunManager: Don't scan the filesystem if autorun is disabled Content-Type scanning can be expensive, and the user disabled autorun, so...
Created attachment 224702 [details] [review] autorunManager: Don't try to scan remote filesystems Content-Type scanning can be super expensive. The autorun manager is meant for local filesystems that are plugged into a USB port or similar, not remote NFS or sshfs mounts.
These patches should keep the same invariants as last time. I still don't have a test case to try out automounting, so these are "blind", per se.
They all look good. I'll test them and report. I don't think they're blocking 3.6.0 anyway, it's fine if we get them in 3.6.1
Did you test them?
Just did: JS ERROR: !!! Exception was: TypeError: this._settings is undefined JS ERROR: !!! message = '"this._settings is undefined"' JS ERROR: !!! fileName = '"/opt/gnome/share/gnome-shell/js/ui/components/autorunManager.js"' JS ERROR: !!! lineNumber = '108' JS ERROR: !!! stack = '"([object _private_unknown_GProxyMount])@/opt/gnome/share/gnome-shell/js/ui/components/autorunManager.js:108 wrapper([object _private_unknown_GProxyMount])@/opt/gnome/share/gjs-1.0/lang.js:204 ([object _private_unknown_GProxyMount],true)@/opt/gnome/share/gnome-shell/js/ui/components/autorunManager.js:212 wrapper([object _private_unknown_GProxyMount],true)@/opt/gnome/share/gjs-1.0/lang.js:204 ([object _private_unknown_GUnionVolumeMonitor],[object _private_unknown_GProxyMount])@/opt/gnome/share/gnome-shell/js/ui/components/autorunManager.js:228 wrapper([object _private_unknown_GUnionVolumeMonitor],[object _private_unknown_GProxyMount])@/opt/gnome/share/gjs-1.0/lang.js:204 Anyway, multiple workarounds exists, so this is not a blocker for 3.6.0. There'll plenty of time to test it more thoroughly.
Created attachment 225223 [details] [review] autorunManager: Don't scan the filesystem if autorun is disabled Content-Type scanning can be expensive, and the user disabled autorun, so...
Can I get a review on this for 3.6.1?
Review of attachment 224700 [details] [review]: Yes
Review of attachment 225223 [details] [review]: Looks good.
Review of attachment 224702 [details] [review]: Tested and works fine with a real USB key, but one thing that comes to my mind is: do AFP or MPT (GDaemonMounts) include a GVolume and a GDrive? I guess not, looking at gvfs sources, so this code regresses opening music players and cameras with the right app.
(In reply to comment #25) > Review of attachment 224702 [details] [review]: > > Tested and works fine with a real USB key, but one thing that comes to my mind > is: do AFP or MPT (GDaemonMounts) include a GVolume and a GDrive? I guess not, > looking at gvfs sources, so this code regresses opening music players and > cameras with the right app. AFP is for over the network, and it even has a volume. You're right for MPT, though, so I'll change the patch to err on the side of caution in this case.
Attachment 224700 [details] pushed as 7e496b1 - autorunManager: Clean up mount operations Attachment 225223 [details] pushed as ff9509b - autorunManager: Don't scan the filesystem if autorun is disabled
Created attachment 225696 [details] [review] autorunManager: Don't try to scan remote filesystems Content-Type scanning can be super expensive. The autorun manager is meant for local filesystems that are plugged into a USB port or similar, not remote NFS or sshfs mounts.
Created attachment 225697 [details] [review] autorunManager: Don't try to scan remote filesystems Content-Type scanning can be super expensive. The autorun manager is meant for local filesystems that are plugged into a USB port or similar, not remote NFS or sshfs mounts.
Review of attachment 225697 [details] [review]: OK (I though AFP was Apple Filing Protocol, and used to access iPhones...)
(In reply to comment #30) > Review of attachment 225697 [details] [review]: > > OK > > (I though AFP was Apple Filing Protocol, and used to access iPhones...) It is Apple Filing Protocol. It's not used to access iPhones, though.
# Bug 684093 - Gnome-shell-hotplug-sniffer block access to mounted sshfs filesystem - UNCONFIRMED Attachment 225697 [details] pushed as 0ad739e - autorunManager: Don't try to scan remote filesystems