Repository navigation
[20.14.0][fs] recursive watch on Linux crashes on .close() #53350
Description
Version
20.14.0
Platform
Linux fedora 6.8.11-300.fc40.x86_64 #1 SMP PREEMPT_DYNAMIC Mon May 27 14:53:33 UTC 2024 x86_64 GNU/Linux
Subsystem
internal/fs/recursive_watch
What steps will reproduce the bug?
- Create a folder with
watch.mjs:
import { watch } from "node:fs";
const watcher = watch(import.meta.dirname, { recursive: true });
watcher.on("change", (eventType, filename) => {
// console.log(eventType, filename);
});
watcher.on("error", (e) => {
console.error(e);
});
console.log("Watching for changes...");git initin foldernode watch.mjs- in the same folder, run the following:
for run in {1..100}; do git add watch.mjs && git rm --cached watch.mjs; donethe above command stages and unstages watch.mjs 100 times. you should see watch errors
Error: ENOENT: no such file or directory, stat '/home/avi/projects/watch-crash/.git/index.lock'
at statSync (node:fs:1658:25)
at #watchFile (node:internal/fs/recursive_watch:152:28)
at #watchFolder (node:internal/fs/recursive_watch:129:26)
at FSWatcher.<anonymous> (node:internal/fs/recursive_watch:184:26)
at FSWatcher.emit (node:events:519:28)
at FSWatcher._handle.onchange (node:internal/fs/watchers:215:12)and
Error: ENOENT: no such file or directory, watch '/home/avi/projects/watch-crash/.git/index.lock'
at FSWatcher.<computed> (node:internal/fs/watchers:247:19)
at watch (node:fs:2469:36)
at #watchFile (node:internal/fs/recursive_watch:156:21)
at #watchFolder (node:internal/fs/recursive_watch:129:26)
at FSWatcher.<anonymous> (node:internal/fs/recursive_watch:184:26)
at FSWatcher.emit (node:events:519:28)
at FSWatcher._handle.onchange (node:internal/fs/watchers:215:12)This is because git saves the lock, and removes it immediately.
Sometimes this statSync call fails (recursive_watch:152):
{
const existingStat = statSync(file);
this.#files.set(file, existingStat);
}which could have been avoided by using throwIfNoEntry: false and checking the return value.
the real race happens when the above statSync call succeeds, and then the watch() call (two lines down) fails because the file just got deleted. This can be seen in the second stack trace above. In this scenario, this.#files will be populated with the stats object, but this.#watchers won't have a matching watcher registered in the map.
When calling close() on watcher after the second scenario happened, the following loop in close will fail:
for (const file of this.#files.keys()) {
this.#watchers.get(file).close();
this.#watchers.delete(file);
}as this.#watchers.get(file).close(); will fail when trying to call close() on undefined (watch() call blew up, so nothing in this.#watchers for file).
the error looks like:
TypeError: Cannot read properties of undefined (reading 'close')
at FSWatcher.close (node:internal/fs/recursive_watch:86:31)How often does it reproduce? Is there a required condition?
Pretty consistently.
What is the expected behavior? Why is that the expected behavior?
Don't crash on close().
When querying whether a node exists or not (statSync), use throwIfNoEntry to avoid generated errors.
What do you see instead?
calling close() crashes the process
Additional information
@mcollina you might be interested. looks like your cup of tea.
Activity
jakecastelli commentedon Jun 6, 2024
I am on macOS aarch 64 with node
v20.13.1- I didn't see the same error but I gotfatal: pathspec 'indx.mjs' did not match any fileswhen dofor run in {1..100}; do git add watch.mjs && git rm --cached watch.mjs; done, I assume I will need to test this on Linux.Also I added
console.count('file has been changed')in theon('change')and I gotfile has been changed: 207207 times.AviVahl commentedon Jun 6, 2024
The
for run in {1..100}; do git add watch.mjs && git rm --cached watch.mjs; doneis just a bash one-liner to git stage and unstagewatch.mjs100 times really quickly. This causes the .lock file to be created and deleted quickly many times.jakecastelli commentedon Jun 6, 2024
yes, I did run it as a bash one-liner, and that's what I got from the ttyfatal: pathspec 'index.mjs' did not match any filesedit: I didn't change the file name in the bash one liner script to
index.mjs, after I changed toindex.mjsand it works, but it didn't crash on Mac.AviVahl commentedon Jun 7, 2024
Not sure where the indx.mjs came from. file has to be named watch.mjs for the steps to work.
Alternatively, I've also reproduce this, and additional full crashes (unhandled exceptions) using:
for run in {1..100}; do git init && rm -rf .git; doneabove will initialize a git repo in the current directory and remove it right away 100 times.
make sure you do it all in a new empty directory.
jakecastelli commentedon Jun 7, 2024
Sorry for the confusion, I renamed
watch.mjstoindex.mjson my end, but apart from that everything else should be the same.I tested
for run in {1..100}; do git init && rm -rf .git; done, there is no error this time, but I didn't reproduce the crash, probably becausefsimplementation inlibuvfor mac and linux is different.I tested on version node
v20.14mac m1.Will see what other people reproduce on this one.
AviVahl commentedon Jun 7, 2024
I believe It has to be Linux to be using the internal recursive_watch.js (and see the issue)
mcollina commentedon Jun 14, 2024
Here is a fix: #53452