search

gmethvin / directory-watcher

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

DirectoryWatcher.watch() throws ClosedWatchServiceException when Watcher is closed #79

Closed kwin closed 2 years ago

kwin commented 2 years ago

According to the javadoc of DirectoryWatcher.watch() (https://github.com/gmethvin/directory-watcher/blob/9eac61c0819db9ce67ac11e2b2c3927621c0fb8a/core/src/main/java/io/methvin/watcher/DirectoryWatcher.java#L224) it

Block(s) until either the listener stops watching or the DirectoryWatcher is closed.

Actually the methods throws a java.nio.file.ClosedWatchServiceException in case the watcher has been closed.

 java.nio.file.ClosedWatchServiceException
    at io.methvin.watchservice.AbstractWatchService.check(AbstractWatchService.java:94)
    at io.methvin.watchservice.AbstractWatchService.take(AbstractWatchService.java:86)
    at io.methvin.watchservice.MacOSXListeningWatchService.take(MacOSXListeningWatchService.java:38)
    at io.methvin.watcher.DirectoryWatcher.watch(DirectoryWatcher.java:231)
....

Either this should be clarified in the javadoc or closing the watcher should just unblock watch without throwing an exception

gmethvin commented 2 years ago

I think it makes more sense to throw immediately if the watcher is closed. What do you think? Happy to accept a pull request to clarify docs.

kwin commented 2 years ago

IMHO just returning without throwing makes more sense. Otherwise I think watch can never return ordinarily. Exceptions should IMHO not be used for regular flow control and closing the watcher is IMHO the only means to end a dedicated watcher thread.

gmethvin commented 2 years ago

ok, I think I understand now. I agree that if watch() is currently watching and the directory watcher is closed, we should return normally. But if you call watch() on a watcher that is already closed, then it should throw, since calling watch on a closed watcher doesn't make sense.

gmethvin commented 2 years ago

see #80