rjeczalik / notify

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

Fixes for recursive tree watcher #208

Open FabianKramm opened 3 years ago

FabianKramm commented 3 years ago

Hello! This PR fixes 3 problems we encountered in the recursive tree watcher implementation and adds 2 new tests:

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")
ch2 := watch("./a/b/c")

// unwatch ./a/b/c -- this is what triggers unwatching of ./a/b as well
notify.Stop(ch2)

// fire an event that will never show up because the path a/b is now unwatched
go func() { ioutil.WriteFile("a/b/c/d", []byte("X"), 0664) }()

// Never terminates
fmt.Println(<-ch1)

}


I added a new test called `TestStopChild` that tests explicitly for this problem.
- The second problem was already described in #155, but is as well a problem in the recursive tree watcher implementation that does not correctly check children nodes for inactive watchpoints. I added a new test called `TestAddParentAfterStop` that tests explicitly for this problem. With the fixes introduced in this PR, the code snippet in #155 runs fine without any panics
- The third problem is that on calling `Stop()`, `RecursiveUnwatch` is never called, because `watchIsRecursive(nd)` is called after the watchpoint was already removed and hence the check always returns that the watch is not recursive