@bboreham thanks for the PR - I'm investigating a similar issue (deadlock on windows) - I have it not yet pinned down but it might be because of fsnotify, too.

However, when testing out your changes I didn't see any behavior changes. As I'm not sure yet if it's really fsnotify in our case, take my words with a grain of salt.
When going through the new code, I see however a bit of a discrepancy in how windows vs non-windows is treated and this change is not enough to fix the issue.

NewWatcher creates a unbuffered Events channel and calls newBackend:

In non-windows systems (E.g. inotify), newBackend calls newBufferedBackend with size 0:

In windows systems however, it calls newBufferedBackend with size 50:

So in the case of windows, when calling fsnotify.NewWatcher, it has an unbuffered Events channel, but windows calls newBufferedBackend with size 50 (even though the sz param is not used in that function).

So IMO the creation of the Events channel should not be done in NewWatcher but instead individually in newBufferedBackend.

What is your take on this?

It seems to me you are proposing a change to the design and/or API. I didn't spend enough time on this to make such judgements.

I was reporting #656, and found a workaround which was defeated by this simpler problem.

However in the project where fsnotify is used, we pinned to the older version 1.7.

@bboreham maybe it wasn't clear what my intention was then:

• the <=1.7 code used a buffered channel of size 50 for windows in all cases (using NewWatcher or using NewBufferedWatcher)
• the 1.8 code uses a buffered channel only if using NewBufferedWatcher

This is because the creation of the Events channel (with/without buffer) is in the platform-agnostic fsnotify.go and only the backend creation is platform-specific.

Therefore, this fix here is valid for users using NewBufferedWatcher but doesn't fix users using NewWatcher on windows. This is a breaking change from the previous version and seems to deadlock even when merging this change.

doesn't fix users using NewWatcher

Agreed. I did not claim this PR fixes any issue, especially not #656.

Thanks; I updated the PR to also a buffer size of 50 by default like before, and added a test for the lot.