inotify: Watcher.Remove may deadlock when multiple fs events occur

[jerryz920]

Before reporting an issue, please ensure you are using the latest release of fsnotify.

Which operating system (GOOS) and version are you using?

Distributor ID: Ubuntu
Description: Ubuntu 14.04.1 LTS
Release: 14.04
Codename: trusty
go version: go1.7.3 linux/amd64

Please describe the issue that occurred.

I run into this issue, and from issue list I think people seeming to have similar problem, so I spend some time digging the cause.

The code I am using is latest commit fd9ec7d
Assuming select loop for events is sequential, see code example below

Inotify may generate multiple events at same time. When this happens, if one happens to call Watcher.Remove on any valid watch path (e.g. for error handling), then it will block on the Watcher's condition variable (inotify.go:158), waiting for a broadcast on sender side (inotify.go:299). However, the channel size is 1, so it will also block waiting for the select loop to consume events, which is already in waiting state.

I tried to increase the event channel size, which allows sender side to proceed, or alternatively one could use goroutine for Remove in order to avoid the deadlock. But I am not sure if this is ideal.

Are you able to reproduce the issue? Please provide steps to reproduce and a code sample if possible.

Code below is to reproduce the issue: a test folder "tests" has a file "file1" inside, and the code tries to rename "tests/file1" to "file2" (which generates two events). If Remove is called inside select loop, it will cause deadlock alarm. Changing watcher.Remove to go watcher.Remove or modify watcher's Events channel would fix it.

package main

import (
        "log"
        "os"
        "time"

        "github.com/fsnotify/fsnotify"
)

func main() {
        watcher, _ := fsnotify.NewWatcher()
        watcher.Add("tests")
        watcher.Add("tests/file1")

        done := make(chan bool)
        first := true

        go func() {
                for {
                        select {
                        case event := <-watcher.Events:
                                log.Printf("event name: %s", event.String())
                                if first {
                                        // *boom*
                                        watcher.Remove("tests/file1")
                                        first = false
                                }
                                //log.Printf("is this solved?")
                        }
                }
        }()

        time.Sleep(2 * time.Second)
        os.Rename("tests/file1", "file2")
        <-done
}

[aarondl]

Have the same issue here. Adding a goroutine really isn't a feasible alternative here since you need to check for errors and the goroutine that owns the watcher/list of watched directories (because we have no recursive API) needs to know when this goroutine succeeds or fails in order to go ahead with deletion from the watched dirs. This really complicates implementing the recursive watcher :(

[nullbio]

Having this problem as well, please see above reference. What can we do about this?

[nullbio]

Same issues: #123 and #115