rjeczalik / notify

File system event notification library on steroids.
MIT License
902 stars 128 forks source link

FSEvents: fix bug where rewatching path fails across channels #155

Open lsegal opened 6 years ago

lsegal commented 6 years ago

The following code reproduces a bug where if you watch a parent and child directory across multiple channels and then call notify.Stop() on the either channel, any subsequent calls to notify.Watch() on a grandparent directory will fail:

package main

import (
    "fmt"
    "io/ioutil"
    "os"

    "github.com/rjeczalik/notify"
)

func watch(path string) chan notify.EventInfo {
    c := make(chan notify.EventInfo, 1)
    if err := notify.Watch(path+"...", c, notify.All); err != nil {
        panic(err)
    }
    return c
}

func main() {
    os.MkdirAll("./a/b/c", 0775)
    defer os.RemoveAll("./a")

    // watch a child and parent path across multiple channels.
    // this can happen in any order.
    ch1 := watch("./a/b/c")
    ch2 := watch("./a/b")

    // unwatch ./a/b -- this is what causes the panic on the next line.
    // note that this also fails if we notify.Stop(ch1) instead.
    notify.Stop(ch2)

    // watching ./a will now return errNotWatched.
    ch3 := watch("./a")

    // just a test to make sure watching still works when we get here.
    go func() { ioutil.WriteFile("a/b/c/d", []byte("X"), 0664) }()
    fmt.Println(<-ch1, <-ch3)
}

Fortunately we can fix this failure by simply not returning errNotWatched from unwatch(), which simply performs a no-op if there is no active stream for the path when Unwatch() is called.

lsegal commented 6 years ago

Any updates on this? The test failure looks like a temporal issue.

rjeczalik commented 6 years ago

Hey @lsegal! Sorry for letting this hang so long, but I'm not sure it's a proper fix - the tree implementation should not try to unwatch something that was already unwatched, thus patching fsevents only would workaround this problem for macOS. If there's a bug, it should be fixed one layer above, but I did not look into this so I can't tell for sure.