Skip to content

fix(inotify): racy watches map access in readEvents #664

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.

Sign up for GitHub

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

Jump to bottom
Closed
FrankSpitulski wants to merge 1 commit into from
Closed

fix(inotify): racy watches map access in readEvents #664

FrankSpitulski wants to merge 1 commit into from
+4 −1

Conversation

@FrankSpitulski
Copy link

@FrankSpitulski FrankSpitulski commented Dec 31, 2024

the go test race detector detected racy access around this map. project specific details omitted.

TLDR of the test is there were a lot of concurrent dir creations, file writes, deletions, and watch.Adds.

WARNING: DATA RACE
Write at 0x00c00412c870 by goroutine 6975:
  runtime.mapassign_faststr()
      /usr/local/go/src/runtime/map_faststr.go:203 +0x0
  github.com/fsnotify/fsnotify.(*watches).updatePath()
      /go/pkg/mod/github.com/fsnotify/[email protected]/backend_inotify.go:163 +0x2b8
  github.com/fsnotify/fsnotify.(*inotify).register()
      /go/pkg/mod/github.com/fsnotify/[email protected]/backend_inotify.go:335 +0x1bd
  github.com/fsnotify/fsnotify.(*inotify).add()
      /go/pkg/mod/github.com/fsnotify/[email protected]/backend_inotify.go:331 +0xee
  github.com/fsnotify/fsnotify.(*inotify).AddWith()
      /go/pkg/mod/github.com/fsnotify/[email protected]/backend_inotify.go:296 +0x405
  github.com/fsnotify/fsnotify.(*inotify).Add()
      /go/pkg/mod/github.com/fsnotify/[email protected]/backend_inotify.go:253 +0x44
  github.com/fsnotify/fsnotify.(*Watcher).Add()
      /go/pkg/mod/github.com/fsnotify/[email protected]/fsnotify.go:313 +0x1929
  ...

Previous read at 0x00c00412c870 by goroutine 6974:
  runtime.mapaccess2_faststr()
      /usr/local/go/src/runtime/map_faststr.go:108 +0x0
  github.com/fsnotify/fsnotify.(*inotify).readEvents()
      /go/pkg/mod/github.com/fsnotify/[email protected]/backend_inotify.go:534 +0xd04
  github.com/fsnotify/fsnotify.newBufferedBackend.gowrap1()
      /go/pkg/mod/github.com/fsnotify/[email protected]/backend_inotify.go:195 +0x33

Goroutine 6975 (running) created at:
  ....(*Monitor).Start()
...
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:1742 +0x44

Goroutine 6974 (running) created at:
  github.com/fsnotify/fsnotify.newBufferedBackend()
      /go/pkg/mod/github.com/fsnotify/[email protected]/backend_inotify.go:195 +0x3c6
  github.com/fsnotify/fsnotify.newBackend()
      /go/pkg/mod/github.com/fsnotify/[email protected]/backend_inotify.go:174 +0x51
  github.com/fsnotify/fsnotify.NewWatcher()
      /go/pkg/mod/github.com/fsnotify/[email protected]/fsnotify.go:253 +0x42
  ....(*Monitor).Start()
      ...
      /usr/local/go/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:1742 +0x44

@stevenctl
Copy link

stevenctl commented Feb 14, 2025

fixes #672

arp242 added a commit that referenced this pull request Mar 20, 2025
@arp242
Fix race in inotify backend
3cf6651 
It would access "watch.wd", but that's racy. I redid quite a bit of the
inotify backend to add some new features, so regression from one of
those changes (and/or would work "by accident" before).

Fixes #653
Fixes #664
Fixes #672
@arp242 arp242 mentioned this pull request Mar 20, 2025
Merged
arp242 added a commit that referenced this pull request Mar 21, 2025
@arp242
Fix race in inotify backend
7007419 
It would access "watch.wd", but that's racy. I redid quite a bit of the
inotify backend to add some new features, so regression from one of
those changes (and/or would work "by accident" before).

Fixes #653
Fixes #664
Fixes #672
arp242 added a commit that referenced this pull request Mar 28, 2025
@arp242
Fix race in inotify backend
894b808 
It would access "watch.wd", but that's racy. I redid quite a bit of the
inotify backend to add some new features, so regression from one of
those changes (and/or would work "by accident" before).

Fixes #653
Fixes #664
Fixes #672
@arp242 arp242 closed this in #678 Mar 31, 2025
@arp242 arp242 closed this in 9a268ac Mar 31, 2025
jakubno pushed a commit to e2b-dev/fsnotify that referenced this pull request Jul 31, 2025
@arp242 @jakubno
Fix race in inotify backend
ac34e36 
It would access "watch.wd", but that's racy. I redid quite a bit of the
inotify backend to add some new features, so regression from one of
those changes (and/or would work "by accident" before).

Fixes fsnotify#653
Fixes fsnotify#664
Fixes fsnotify#672
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@FrankSpitulski @stevenctl