-
Notifications
You must be signed in to change notification settings - Fork 5.1k
FileSystemWatcher.Linux: use a single inotify instance and refactor watch tracking. #117148
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
base: main
Are you sure you want to change the base?
Conversation
@dotnet/area-system-io @stephentoub ptal. |
src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs
Outdated
Show resolved
Hide resolved
…-Windows configurations.
… or we won't update _wdToWatch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tmds big thanks for your contribution! For now I've reviewed 25% of this PR (I need to wrap my head around all the locks and it's going to take me a while), PTAL at my comments.
src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs
Outdated
Show resolved
Hide resolved
For the locking, this overview may be helpful:
To prevent deadlocks, the locks (as needed) are taken in this order: watchersLock, addLock, lock on Watcher, lock on Watch. |
@adamsitnik this is a challenge to review so feel free to ask any questions you have while looking at the code. I'm also going to make some time this week to look at the PR with my reviewer's hat on. |
(By default) Linux allows 128 inotify instances per user. By sharing the inotify instance between the FileSystemWatchers we reduce contention with other applications.
Fixes #62869.