Conversation
There was a TOCTOU inversion here causing this to be a datarace. The time needs to be looked up in advance of looking at any mtime.
| stillwatching = haskey(watched_files, dirname) | ||
| if stillwatching | ||
| wf = watched_files[dirname] | ||
| timestamp = updatetime!(wf) |
There was a problem hiding this comment.
Doing this unconditionally (and/or this early) seems to break the test added in #342.
Seems like this should maybe just be timestamp = wf.timestamp, keeping the original line 337 intact.
There was a problem hiding this comment.
Doing this conditionally in any form is a TOCTOU data race and unsound
There was a problem hiding this comment.
okay, I see what the test wants to do though, and what the test wants is not what this function is currently capable of doing. The only way to make that test pass reliably is to track changes to the mtime+inode+size triple on a individual file bases, and not use updatetime at all
There was a problem hiding this comment.
Changed to use ctime instead, so that any metadata change since the last scan (such as a rename) will also trigger a full re-inspection of the contents
2 participants
There was a TOCTOU inversion here causing this to be a datarace. The time needs to be looked up in advance of looking at any mtime.