[File Watchers] Can not externally rename a directory with subfolders on Windows

[jasonsanjose]

UPDATE: I don't remember seeing this earlier, but now if I load a simple tree 2 levels deep...

ProjectRoot
----folder
--------subfolder
------------file.txt

...and I try to rename folder externally, I get the "folder in use" dialog.

Old Description:

While debugging #6537, I found the following...

If, external to brackets, I copy in a folder with a subfolder into my project, then externally try to rename the folder that I pasted, windows prevents me from doing so with this warning:

Folder In Use
The action can't be completed because the folder or a file in it is open in another program. Close the folder or file and try again.

Note that this only happens for externally added folders that include a subfolder. Pasting in a folder with no subfolders doesn't reproduce the issue. I assume that the file watcher for the subfolder is what is holding a file lock.

This may be a sprint 36 blocker.

cc @iwehrman @peterflynn @gruehle

[jasonsanjose]

@peterflynn made a good point, why doesn't this happen for all watched folders? Indeed it doesn't...

[iwehrman]

So Windows doesn't like the rename? Is the hypothesis that the Node watcher is somehow locking the directory? I have no idea how Windows filesystem locking works. @peterflynn?

[dangoor]

I ran Process Explorer and it confirmed that brackets-node has a handle on the directory that I moved.

[peterflynn]

@jasonsanjose @iwehrman I have a theory that the stuck handle is actually a result of the watcher notification from the directory getting added while its parent was already watched. That would explain why it only occurs for added directories and not all directories. Does anything in the Node-side code fit into that picture? Or could there be a bug in the Node watcher impl itself...?

[jasonsanjose]

Not exactly sure what you mean about the notification. Can you clarify?

[jasonsanjose]

I don't remember seeing this earlier, but now if I load a simple tree 2 levels deep...

ProjectRoot
----folder
--------subfolder
------------file.txt

...and I try to rename folder externally, I get the "folder in use" dialog.

[jasonsanjose]

@iwehrman found the node bug, it looks like an FOL nodejs/node-v0.x-archive#3963.

[jasonsanjose]

Tested with Node 0.11.10 and the problem still exists.

[iwehrman]

The available options are unpalatable:

1. Live with the bug on Windows;
2. Disable watchers on Windows, patching up the caching infrastructure to always cache directory contents, and to invalidate directory contents on window focus if unwatched; or
3. Use Node's slower, more expensive fs.watchFile operation (or one of its many wrapper libraries) for file-change notifications.

The best option in my opinion is 2. The downside is that it will require some additional code. The upside is that it will probably leave the filesystem code in a better state overall, since there's no particular reason why we shouldn't have the proposed caching policy already for unwatched directories except for the added complexity.

Side note: although the aforementioned Node issue would indicate that this is impossible on Windows, I frankly don't believe that. I bet it works in Sublime somehow or another. It would be nice to know how.

CC @gruehle

[peterflynn]

This is confusing. If I have Sublime open, it lets me rename folders with folder children, yet it still detects the rename instantly.

Windows Explorer itself it obviously also able to do this. So it seems like there's some Windows API that has the behavior we want...

Could they really both be doing the super inefficient one-watcher-per-file thing??

[iwehrman]

Because Explorer can do it efficiently, it seems very likely that the answer is that there exists an API that libuv doesn't use. Also, if they're spinning up a watcher-per-visible-file, they still need a way to watch for new files...

[peterflynn]

We've identified at least four promising-sounding options that might allow efficient watching without locking most (or any) folders... It does seem like there's a good chance libuv is simply 'doing it wrong' -- Windows isn't really a top-priority platform for Node, and the main native watching API is apparently pretty complicated to use effectively:

E.g., from http://qualapps.blogspot.com/2010/05/understanding-readdirectorychangesw.html

The biggest challenge to using ReadDirectoryChangesW is that there are several hundred possibilities for combinations of I/O mode, handle signaling, waiting methods, and threading models. Unless you're an expert on Win32 I/O, it's extremely unlikely that you'll get it right, even in the simplest of scenarios.

So at this point we just need to proove out which alternative is going to work well, and then start thinking about how that fits into our desire to ship a Brackets update relatively soon...

[jasonsanjose]

FYI this was also tested on Mac and Linux...both are unaffected.

[bchintx]

Pull requests #6673 and #412 should fix this issue. Per @peterflynn 's note, @JeffryBooher @ingorichter and I have implemented a native node add-on for Windows, called 'fsevents_win'.

This new node-module replaces Brackets' previous use of node's fs.watch() with a Windows-native implementation of the same ReadDirectoryChangesW() function, but leveraging the 'recursive' flag. So, on Windows, we now only have to set a single watcher at the project root level, rather than having to iterate thru and set individual fs.watch() watchers for each subfolder within the project. As a result, the observed "in-use" issues disappear.

Per @jasonsanjose 's last comment, the reason why this didn't occur on Mac was because @iwehrman had already integrated a similar 3rd-party node-module that provided this same native functionality on the Mac.

FYI... one side benefit of only setting a watcher on the project's root folder is that we now also lock that folder so that it can't be deleted or renamed out from under us while it's open within Brackets. :-)

[ackalker]

Does anyone have some smart comments on http://timgolden.me.uk/python/win32_how_do_i/watch_directory_for_changes.html ?
Especially the part following the use of the ReadDirectoryChanges API marked "Update:" which suggests that renaming a watched directory should be possible (i.e. without the 'watch the parent' trick).

[bchintx]

@ackalker Good observation. The comment referred to in that link points to how using FILE_SHARE_DELETE in the CreateFile() call will keep the code from locking the folder that you're wanting to watch with ReadDirectoryChangesW(). However, even if you use that flag, the parent folder of the watched folder will be locked by the ReadDirectoryChangesW() call.

So, in this implementation, we've chosen to not use this flag, and so Brackets will be locking the root of the project folder (and its parent folder). However, since we are now able to recursively watch all subfolders with a single-watcher, we've fixed the problem of not being able to externally rename/delete folders within the project itself.

(IMHO, it's actually a positive that we're now locking the root of the project folder so that it won't be moved/deleted out from underneath us as easily.)

[bchintx]

The Pull Requests have been merged into the release and master branches now, so marking this as FBNC back to @jasonsanjose

[jasonsanjose]

Confirmed fix on release build 0.37.0-11591 (build number to be fixed soon right?)