gmethvin / directory-watcher

A cross-platform Java recursive directory watcher, with a JNA macOS watcher and Scala better-files integration
Apache License 2.0
265 stars 34 forks source link

No events are emitted when a folder with a file inside is moved into another folder #10

Closed jkawamoto closed 6 years ago

jkawamoto commented 6 years ago

DirectoryWatcher is watching C:\Users\jun_w\Goobox\ and the folder has New folder and New folder (2), and New folder has New Text Document.txt, i.e.,

Goobox\
   ├ New folder\
   │   └ New Text Document.txt 
   └ New folder (2)\

When I move New folder to New folder (2), I expected ENTRY_CREATE or ENTRY_MODIFY were emitted to New folder and New Text Document.txt but I got only events for New folder.

Here is log entries I got:

DEBUG io.methvin.watcher.DirectoryWatcher - ENTRY_DELETE [C:\Users\jun_w\Goobox\New folder] 
DEBUG io.methvin.watcher.DirectoryWatcher - ENTRY_DELETE [C:\Users\jun_w\Goobox\New folder] 
DEBUG io.methvin.watcher.DirectoryWatcher - ENTRY_CREATE [C:\Users\jun_w\Goobox\New folder (2)\New folder] 
DEBUG io.methvin.watcher.DirectoryWatcher - ENTRY_MODIFY [C:\Users\jun_w\Goobox\New folder (2)] 

This problem didn't happen in macOS 10.13.3.

gmethvin commented 6 years ago

I think you could make a case for either behavior. The "move" isn't really doing anything to the file inside, so I suspect that's why it doesn't treat that as a "create" event for the file. Note that on Windows we actually use the native JDK implementation to watch recursively, so this is actually just what the native implementation does and not a specific behavior I decided on myself.

Did you check what this does on Linux? If it's consistent with Windows I'd be more inclined to look at changing the Mac behavior. But either way it'd be nice if the behavior was consistent between OSes.

jkawamoto commented 6 years ago

Thank you for your prompt response.

I guess Windows emits ENTRY_CREATE event for New Text Document.txt before New folder is registered in the watch service, and hence the watch service cannot notify the event.

DEBUG io.methvin.watcher.DirectoryWatcher - ENTRY_CREATE [C:\Users\jun_w\Goobox\New folder] 
DEBUG io.methvin.watcher.DirectoryWatcher - ENTRY_MODIFY [C:\Users\jun_w\Goobox\New folder\popular_book_review.json] 
DEBUG io.methvin.watcher.DirectoryWatcher - ENTRY_MODIFY [C:\Users\jun_w\Goobox\New folder] 

When New folder had a bigger file (popular_book_review.json is about 150MB), ENTRY_MODIFY event for the file was notified. I think ENTRY_CREATE was emitted, then New folder was registered, and ENTRY_MODIFY was emitted, so only ENTRY_MODIFY was notified.

However, it's impossible to register New folder in advance and I agree current behavior may be intended by the JDK implementation.

Unfortunately, since our software doesn't support Linux yet, I haven't checked this issue in Linux.

gmethvin commented 6 years ago

I'm curious which filesystem actions are causing those log entries. Is that from copying "New folder" from somewhere else?

jkawamoto commented 6 years ago

Yes, it is. I moved "New folder" which has a large file "popular_book_review.json" from outside of the watching folder but in the same drive.

gmethvin commented 6 years ago

It's a bit disappointing if Windows has a race condition like you described. I haven't looked at their actual implementation but I thought the whole point of the FILE_TREE modifier was to tell Windows you wanted all events for the entire tree.

The only way we could work around this is to traverse the entire file tree of a new directory when it's created. I'm open to potentially doing that but it could be a costly operation in some cases.

In most of my use cases of this library the race condition is not really an issue, since the application would be traversing the directory anyway to do whatever it needed to do (e.g. compiling code that changed).

jkawamoto commented 6 years ago

I'm trying to traverse the directory when ENTRY_CREATE is emitted with a directory, and it works so far.

If this library traverses the directory instead of the user, it'd be helpful. If I'm not mistaken, registerAll traverses the directory. So if ENTRY_CREATE event for descendants can be emitted in that traverse, it seems not so costly. What do you think?

But, in my case, such double traversing the directory is not costly so far even I copied thousands files. I think it might be enough to mention about missing events in the doc.

gmethvin commented 6 years ago

registerAll shouldn't actually traverse the whole tree on Windows though, since Windows supports the ExtendedWatchEventModifier.FILE_TREE modifier. DirectoryWatcher should detect that and only register the root.

Anyway, I do think it makes sense to emit extra events for descendants on ENTRY_CREATE as long as we can do so consistently across platforms. A PR doing this would be welcome.

jkawamoto commented 6 years ago

It seems ironical because if the file tree is supported, walk directories to notify create events, and if the file tree isn’t supported, walk directories to register subdirectories….

gmethvin commented 6 years ago

We probably also want to walk the directories to create events when FILE_TREE is not supported. In that case we know we could have missed events between the time the parent directory was created and the time we start listening. In either case we should be able to check if we've already hashed the file to avoid creating multiple events.