-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
go.exp/fsnotify: data race #8282
Labels
Comments
Does this CL solve the data race for you? https://golang.org/cl/103300045/ |
Could this be an issue with readEvents() blocking? The example included with fsnotify reads events off the channel in a separate goroutine: https://code.google.com/p/go/source/browse/fsnotify/example_test.go?repo=exp Whereas your play example doesn't use a separate goroutine. Also see: howeyc/fsnotify#7 |
> Could this be an issue with readEvents() blocking? I just may have spotted a wrong mutex use: at fsnotify_bsd.go:120 the w.pmut is locked to get a copy of w.watches (which is fine) but the pmut "Protects access to paths and finfo" and the wmut "Protects access to watches." So the wmut should have been used unless I'm reading it all wrong. The pmut use is present in howeyc/fsnotify as well. > The example included with fsnotify reads events off the channel in a separate goroutine: Ok, it may have been an intended api and it doesn't make my day any different. http://play.golang.org/p/UZr1ODb8ja The code again does one thing: receives an event and closes and exits, and I cannot comprehend the difference between the two plays BUT running the new one with howeyc/fsnotify no data races observed. with os/fsnotify the exactly same data race present. So that's a difference between the two packages. The issue indeed may have been fixed (with the CL changes) in howeyc/fsnotify. For now using howeyc/fsnotify with a separate goroutine does it for me. |
Thanks for identifying the incorrect mutex. I've opened a pull request to howeyc/fsnotify howeyc/fsnotify#100 Eventually I will get a CL to go.exp as well (I'm not familiar enough to open multiple CLs at once, so waiting for review of https://golang.org/cl/105370044/). |
This fix was merged into fsnotify some time ago: https://github.com/go-fsnotify/fsnotify/blob/master/CHANGELOG.md#dev--2014-07-04 More recently I've reworked kqueue to only use a single mutex. I'm no longer maintaining go.exp/fsnotify: https://groups.google.com/forum/#!msg/golang-dev/-__vD-kOF5s/ Feel free to close this issue. |
This issue is long gone. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The text was updated successfully, but these errors were encountered: