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

Directory watcher freaks out on recursive symlinks #28

Closed lihaoyi closed 5 years ago

lihaoyi commented 5 years ago

On OS-X, starting in an empty folder, setting up a watcher:

@ {
  import $ivy.`io.methvin:directory-watcher:0.9.0`
  import io.methvin.watcher.DirectoryWatcher
  val watcher = DirectoryWatcher
        .builder
        .fileHashing(false)
        .path(java.nio.file.Paths.get("."))
        .listener{ event => println("EVENT " + event.path())}
        .build

  watcher.watchAsync()
}

Followed by a recursive symlink:

$ mkdir folder
$ ln -s ../folder folder/link

Causes a bajillion events to fire:

EVENT /Users/lihaoyi/test/folder
EVENT /Users/lihaoyi/test/folder/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link/link/link/link/link/link/link/link/link/link/link/link/link
EVENT /Users/lihaoyi/test/folder/link

I'm not sure if this a property of the underlying OS-X filesystem API, or part of this library. Is there any way to make it not do that?

lihaoyi commented 5 years ago

Seems it's a problem in the Java layer. Creating a recursive symlink when watching using the raw JNA API has no issue:

@ {
  import $ivy.`io.methvin:directory-watcher:0.9.0`
  import com.sun.jna.{NativeLong, Pointer}
  import io.methvin.watcher.DirectoryWatcher
  import io.methvin.watchservice.jna.{CFIndex, CFStringRef, CarbonAPI, FSEventStreamRef}
  import io.methvin.watchservice.jna.CarbonAPI.FSEventStreamCallback

  val streamRef = CarbonAPI.INSTANCE.FSEventStreamCreate(
    Pointer.NULL,
    new FSEventStreamCallback{
      def invoke(streamRef: FSEventStreamRef,
                 clientCallBackInfo: Pointer,
                 numEvents: NativeLong,
                 eventPaths: Pointer,
                 eventFlags: Pointer,
                 eventIds: Pointer) = {
        val length = numEvents.intValue
        val arr = eventPaths.getStringArray(0, length)
        println("FSINVOKE " + arr.toSeq)
      }
    },
    Pointer.NULL,
    CarbonAPI.INSTANCE.CFArrayCreate(
      null,
      Array[Pointer](CFStringRef.toCFString(os.pwd.toString()).getPointer()),
      CFIndex.valueOf(1),
      null
    ),
    -1,
    0.01,
    0
  )
  CarbonAPI.INSTANCE.FSEventStreamScheduleWithRunLoop(
    streamRef,
    CarbonAPI.INSTANCE.CFRunLoopGetCurrent(),
    CFStringRef.toCFString("kCFRunLoopDefaultMode")
  )
  CarbonAPI.INSTANCE.FSEventStreamStart(streamRef)
  CarbonAPI.INSTANCE.CFRunLoopRun()
  }
mkdir folder
ln -s ../folder folder/link
FSINVOKE WrappedArray(/Users/lihaoyi/test/)
FSINVOKE WrappedArray(/Users/lihaoyi/test/folder/)
FSINVOKE WrappedArray(/Users/lihaoyi/test/folder/)
gmethvin commented 5 years ago

I agree this is undesirable behavior. Do you expect it to follow the symlinks? I actually don't think it should, at least not by default.

gmethvin commented 5 years ago

Actually it's not too hard to stop when we hit a cycle. I think that's better since we currently do follow symlinks and I don't want to break anyone relying on this behavior.

lihaoyi commented 5 years ago

For my current use case, I don't want to recurse into symlinks at all. Perhaps you could make it configurable? Similar to how most java.nio APIs take an optional FOLLOW_LINKS and NOFOLLOW_LINKS flag. At the very least it should stop on infinite recursion.

I also ended up using the raw CarbonAPI, rather than relying on your Watcher implementation. It's not clear to me why the recursion here is necessary, since FSEventStreamCreate already registers events for any file/folder recursively within the target path; at most you should only need a shallow listing to figure out which file changed

gmethvin commented 5 years ago

Actually the immediate fix for this is probably not to follow symlinks, because the existing macOS recursive watching functionality won't follow them when reporting changes. I could add support for it in the future perhaps, but it'd take more work to do it properly.