Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for recursive tree watcher #208

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

FabianKramm
Copy link
Contributor

@FabianKramm FabianKramm commented Jul 19, 2021

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

  • In the recursive tree implementation stopping a child watch will actually unwatch the parent as well, this can be reproduced with the following snippet on MacOS or Windows which uses the recursive tree implementation:
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")
	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 FSEvents: fix bug where rewatching path fails across channels #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 FSEvents: fix bug where rewatching path fails across channels #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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant