GNOME Bugzilla – Bug 532911
stats bookmarks on startup ...
Last modified: 2011-08-14 15:06:58 UTC
Running a quick strace of nautilus; it appears to load & -synchronously- stat/access the contents of ~/.gtk-bookmarks on startup :-) that, of course, is not always a good idea; it's quite easy to get a stale NFS mount in your ~/.gtk-bookmarks, which then will give an ~infinite timeout on startup. Ideally that work could be done asynchronously; or deferred until the bookmark menu is selected, or some combination of both ;-)
strace: 27340 1210590327.589732 access("/home/michael/.nautilus", F_OK) = 0 27340 1210590327.589789 mkdir("/home/michael/.nautilus/metafiles", 0700) = -1 EEXIST (File exists) 27340 1210590327.589944 access("/opt/OpenOffice/go-oo.org/go-oo.org", F_OK) = 0 27340 1210590327.590090 access("/home/michael/.nautilus", F_OK) = 0 27340 1210590327.590138 mkdir("/home/michael/.nautilus/metafiles", 0700) = -1 EEXIST (File exists) 27340 1210590327.590223 access("/opt/OpenOffice/HEAD/patches/src680", F_OK) = 0 27340 1210590327.590383 access("/home/michael/.nautilus", F_OK) = 0 27340 1210590327.590429 mkdir("/home/michael/.nautilus/metafiles", 0700) = -1 EEXIST (File exists) 27340 1210590327.590511 access("/media/disk", F_OK) = -1 ENOENT (No such file or directory) 27340 1210590327.590555 access("/media/disk", F_OK) = -1 ENOENT (No such file or directory) 27340 1210590327.590687 access("/home/michael/.nautilus", F_OK) = 0 27340 1210590327.590750 mkdir("/home/michael/.nautilus/metafiles", 0700) = -1 EEXIST (File exists) 27340 1210590327.590833 access("/home/demo/Desktop/Xgl", F_OK) = -1 ENOENT (No such file or directory) 27340 1210590327.590883 access("/home/demo/Desktop/Xgl", F_OK) = -1 ENOENT (No such file or directory) 27340 1210590327.591022 access("/home/michael/.nautilus", F_OK) = 0 27340 1210590327.591069 mkdir("/home/michael/.nautilus/metafiles", 0700) = -1 EEXIST (File exists) 27340 1210590327.591151 access("/home/michael/Documents", F_OK) = 0 27340 1210590327.591452 access("/home/michael/.nautilus", F_OK) = 0 27340 1210590327.591500 mkdir("/home/michael/.nautilus/metafiles", 0700) = -1 EEXIST (File exists) 27340 1210590327.591720 access("/home/michael/.nautilus", F_OK) = 0 27340 1210590327.591767 mkdir("/home/michael/.nautilus/metafiles", 0700) = -1 EEXIST (File exists) 27340 1210590327.591850 access("/home/michael/esc/amd", F_OK) = 0 27340 1210590327.591999 access("/home/michael/.nautilus", F_OK) = 0 27340 1210590327.592054 mkdir("/home/michael/.nautilus/metafiles", 0700) = -1 EEXIST (File exists) 27340 1210590327.592139 access("/home/michael/conferences/2007", F_OK) = 0 27340 1210590327.592277 access("/home/michael/.nautilus", F_OK) = 0 for a gtk-bookmarks of: file:///opt/OpenOffice/go-oo.org/go-oo.org file:///opt/OpenOffice/HEAD/patches/src680 src680 file:///media/disk file:///home/demo/Desktop/Xgl file:///home/michael/Documents x-nautilus-desktop:/// file:///home/michael/esc/amd file:///home/michael/conferences/2007 ...
Created attachment 110844 [details] [review] proposed patch This makes sense to me, attached patch reworks the loading of the list to be async.
wow :-) nice patch & response time Cosimo. /me is no nautilus expert - but do we need to keep a ref-count on the NautilusBookmarkList in case it is freed under our feet before the async callback ? [ or conversely, track & cancel the async load_contents on destruction (?) ]. Then again, perhaps that is an application-lifetime singleton [ not looked ;-]. Thanks.
(In reply to comment #3) > Then again, perhaps that is an application-lifetime singleton [ not looked ;-]. Yeah, AFAICS the bookmark list is initialized once and destroyed only at shutdown, see nautilus-window-bookmarks.c:~50.
Thanks for your efforts! I like the general approach, but the file should really only be loaded once at a time. Assuming you get some subsequent "changed" events, this will cause unneccessary parallel read operations. I also wonder whether CONTENTS_CHANGED is emitted the first time we read the file. How are the bookmark list clients notified? I'd move back the file monitor installation back to nautilus_bookmark_list_init() and always emit CONTENTS_CHANGED when the read is finished.
Created attachment 111175 [details] [review] proposed patch v2 Thanks Christian for the review. Here's a second version of the patch, updated according to your last comment.
Thanks for the fast update. I'd prefer if you always issued a re-read when the file changes, instead of just letting the old update run. Instead, you should cancel the pending load operation.
Created attachment 111512 [details] [review] proposed patch v3 Third version of the patch. Reworks also the save operation to be async. Also, this should hopefully fix bug 530858 too.
Ok - this is also rather worse than I thought. We don't just stat bookmarks on startup - we seem to go stat crazy ;-) and we do this every time we open a new window. So - basically, as soon as you get a bad NFS mount and there is a bookmark there, nautilus hangs. Ultimately though - do we really even need to be statting these things ? surely people can find out when they select it that their bookmark has gone away ? [ as in a web browser eg. ? ]. Here is a fun strace fragment from me trying to open a window having done this: mount -t nfs 192.168.0.8:/opt/share /share ssh 192.168.0.8 sudo /etc/init.d/nfsserver stop # double click on a folder on the desktop to open a new window 2734 1213008790.088460 access("/share/noel", F_OK <unfinished ...> # wait around twiddling thumbs endlessly - eventually re-start nfsserver: 2734 1213008838.092088 <... access resumed> ) = 0 2734 1213008838.092229 access("/share/noel", F_OK) = 0 ... continue & pop-up the window ... So - this is really not ideal ;-)
Ideally we should just store the icon in the bookmark file instead of stating for that info (which i think is all we use it for). That would both solve this problem and allow the right icon for not-currently-mounted bookmarks.
sounds great; though I guess an 'access' syscall is prolly an "is it really there" type check, but ... anything, anything :-)
Cosimo / Christian - did we get this committed in the end ? :-)
ping ping, Hey Nautilus crew, please answer to Mickeal, would be nice to have it commited if not yet. Cheers, -- This message is brought to you by the Bugsquad team.
*** Bug 322507 has been marked as a duplicate of this bug. ***
Sorry for the late response, and thanks for your efforts: Cosimo: I am not sure whether it is wise to use multi-threading for loading and writing the bookmark file. This will have all kinds of issues when not locking the variables. Maybe you should use g_file_load_contents_async() and g_file_replace_async() instead. Otherwise, I like the basic idea but I do not think that bookmark_file_changed_callback should emit a “contents-changed” signal if the underlying bookmark does not use a custom name, because only custom names are stored in ~/.gtk-bookmarks. Also, nautilus_bookmark_set_name should only emit a “contents-changed” signal if a custom name is set. Marking patch as needs work.
I have an updated patch for this in a branch which refactors some of the bookmarks code.
Fixed in master now.
Hi, I'm using Ubuntu Natty (Nautilus 2.32.2.1) and it seems that this bug still applies to me. Please inform me me if the patch has not found the way into this version and you may skip the following. I have NAS hosting the nfs share. On my computer I have an autofs operated nfs mount that invokes a little script which is waking up the NAS. I use a bookmark for conveniently accessing the directory. The idea of this is that the NAS is only powered on if necessary. This works perfectly. But when I login everything hangs, i.e., no icons appear on the desktop and I'm not able to open nautilus. The problem here is that I use network-manager to enable my WLAN. But this is slower than that autofs script sending the wakeonlan message. Now, if I remove the nautilus bookmark to the directory, then this does not happen. I don't see the need to access the directory on boot or opening nautilus. What is the idea of this. That is, what is the intended action when the directory is not accessible. The only time the directory should be accessed should be when the directory is accessed, i.e. when I click on the directory. Regards, Matthias