Skip to content
This repository has been archived by the owner on Jun 8, 2022. It is now read-only.

Fix data race on kevent buffer. #98

Merged
merged 1 commit into from
Jun 17, 2014
Merged

Fix data race on kevent buffer. #98

merged 1 commit into from
Jun 17, 2014

Conversation

tilaks-zz
Copy link

In the BSD implementation of fsnotify, the watcher's kbuf buffers a kevent
between syscall.SetKevent (which prepares the kevent) and syscall.Kevent
(which registers the kevent). The implementation intends to protect access to
kbuf, but fails to do so in addWatch and removeWatch.
This change fixes the data race by allocating a new kevent buffer for every
method invocation.

In the BSD implementation of fsnotify, the watcher's kbuf buffers a kevent
between syscall.SetKevent (which prepares the kevent) and syscall.Kevent
(which registers the kevent). The implementation intends to protect access to
kbuf, but fails to do so in addWatch and removeWatch.
This change fixes the data race by allocating a new kevent buffer for every
method invocation.
@nathany
Copy link
Contributor

nathany commented Jun 10, 2014

@tilaks Thanks for the contribution. Please be sure to sign the CLA https://developers.google.com/open-source/cla/individual?csw=1

Would you be willing to send this patch to go.exp/fsnotify? https://code.google.com/p/go/source/browse?repo=exp#hg%2Ffsnotify

Otherwise I can upload it on your behalf.

@tilaks-zz
Copy link
Author

@nathany thanks, submitted a change for review at https://codereview.appspot.com/103300045

@nathany
Copy link
Contributor

nathany commented Jun 12, 2014

Thanks @tilaks. I haven't had a chance to try out the change, but I'll make some time for that. Someone else will need to submit (merge) the change.

howeyc added a commit that referenced this pull request Jun 17, 2014
Fix data race on kevent buffer.

Not sure why a mutex doesn't work, but this fix will work.
@howeyc howeyc merged commit 13cd45d into howeyc:master Jun 17, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants