Skip to content
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

fix FileWatching designs and add workaround for a stat bug on Apple #55877

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Sep 25, 2024

What started as an innocent fix for a stat bug on Apple (#48667) turned into a full blown investigation into the design problems with the libuv backend for PollingFileWatcher, and writing my own implementation of it instead which could avoid those singled-threaded concurrency bugs.

@vtjnash vtjnash added the domain:io Involving the I/O subsystem: libuv, read, write, etc. label Sep 25, 2024
@vtjnash vtjnash force-pushed the jn/fix-stat-PollingFileWatcher branch 4 times, most recently from 52da641 to ec8d193 Compare September 26, 2024 19:36
@vtjnash vtjnash added the kind:don't squash Don't squash merge label Sep 26, 2024
@vtjnash vtjnash changed the title fix PollingFileWatcher design and add workaround for a stat bug on Apple fix FileWatching designs and add workaround for a stat bug on Apple Sep 26, 2024
@vtjnash vtjnash added the domain:parallelism Parallel or distributed computation label Sep 26, 2024
@vtjnash vtjnash force-pushed the jn/fix-stat-PollingFileWatcher branch from ec8d193 to fb1b40f Compare September 27, 2024 14:14
@vtjnash vtjnash added the status:merge me PR is reviewed. Merge when all tests are passing label Sep 27, 2024
Copy link
Sponsor Member

@timholy timholy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice. May be useful for timholy/Revise.jl#837

stdlib/FileWatching/src/FileWatching.jl Outdated Show resolved Hide resolved
stdlib/FileWatching/src/FileWatching.jl Show resolved Hide resolved
stdlib/FileWatching/src/FileWatching.jl Outdated Show resolved Hide resolved
… stat bug

What started as an innocent fix for a stat bug on Apple (#48667) turned
into a full blown investigation into the design problems with the libuv
backend for PollingFileWatcher, and writing my own implementation of it
instead which could avoid those singled-threaded concurrency bugs.
Previously pidfile used the same poll_interval as sleep to detect if
this code made any concurrency mistakes, but we do not really need to do
that once FileMonitor is fixed to be reliable in the presence of
parallel concurrency (instead of using watch_file).
@vtjnash vtjnash force-pushed the jn/fix-stat-PollingFileWatcher branch from fb1b40f to f8d17e7 Compare September 30, 2024 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:io Involving the I/O subsystem: libuv, read, write, etc. domain:parallelism Parallel or distributed computation kind:don't squash Don't squash merge status:merge me PR is reviewed. Merge when all tests are passing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants