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

bug: on MacOS, fs.watch emits "change" event when access timestamp is updated #49916

Closed
llllvvuu opened this issue Sep 28, 2023 · 6 comments
Closed

Comments

@llllvvuu
Copy link

llllvvuu commented Sep 28, 2023

Version

v20.3.1

Platform

Darwin macbook-air 22.5.0 Darwin Kernel Version 22.5.0: Thu Jun 8 22:22:19 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T8103 arm64

Subsystem

fs

What steps will reproduce the bug?

Contents of repro.js:

const fs = require("fs")

fs.watch("watchee.js", "utf-8", async function (event, fileName) {
  if (event === "change") {
    console.log(`File ${fileName} has been changed`)
  }
})
; touch -a watchee.js
; stat -x watchee.js
; node repro.js
; touch -a watchee.js
; stat -x watchee.js

The stat -x verifies that only the access timestamp changed, not the modified.

How often does it reproduce? Is there a required condition?

Consistently.

What is the expected behavior? Why is that the expected behavior?

The expected behavior is for accesses to either emit a different type of event "access", or no event at all. I believe the latter is the intended behavior, as that's what happens on WSL.

The reason this is expected is because of the meaning of the word "change", as well as the real-world usage of this API (related: microsoft/TypeScript#52876)

What do you see instead?

Repro script prints out File package.json has changed in response to touch -a.

In the wild, this causes files to be rebuilt.

Additional information

This issue doesn't happen on WSL. I haven't tried Windows.

Following Apple's fsevents guide, gives me flags 66560 for touch and no event for touch -a. I tried this with both 0xffffffff and kFSEventStreamCreateFlagNoDefer | kFSEventStreamCreateFlagFileEvents.

Let me try making a debug build of main to see what Node.js does differently.

@bnoordhuis
Copy link
Member

It's "change" because the metadata changed - atime is metadata. The kernel reports kFSEventStreamEventFlagItemModified (0x1000) or kFSEventStreamEventFlagItemInodeMetaMod (0x400) and node/libuv duly reports that.

It's possible the behavior changed in newer macos releases. I only test on an old Intel MBA.

@bnoordhuis
Copy link
Member

Forgot to mention: 66560 is kFSEventStreamEventFlagItemIsFile + kFSEventStreamEventFlagItemModified

@llllvvuu
Copy link
Author

llllvvuu commented Sep 28, 2023

So it turns out I didn't read the docs 😅 :

On macOS, this uses kqueue(2) for files and FSEvents for directories.

So I guess my case is from kqueue. That's why I couldn't reproduce with FSEvents. Unfortunately it seems like atime and mtime changes are distinguished in FSEvents but not in kevent. For build/test system use cases it could make sense to ignore NODE_ATTRIB entirely but probably not for other use-cases 😞

This is tricky. Responding to mtime but not atime is the ideal behavior, which Linux fs.watch() gets, and on MacOS fs.watchFile() does that but is slow. I didn't look into it but presumably FSEvents isn't ideal for files. The last thing I could find on the kevent issue was in 2008: https://lists.apple.com/archives/Darwin-dev/2008/May/msg00041.html

I wonder if this can be solved at a higher level? FSWatcher could fire "attr" | "change" | "rename" instead of "change" | "rename" or even more granular. Not the cleanest change but don't have any better ideas at the moment.

As it is, it seems like the user workaround has to be to stat the files in the user-provided handler. Which might be hard to convince tooling maintainers to do.

@Gonz-coding-co

This comment was marked as off-topic.

@bnoordhuis
Copy link
Member

Ah... your original message made it seem like watchee.js was a directory because of the File package.json has changed message you mentioned.

So, fsevents only works on directories, that's why libuv falls back to kevent/kqueue when watching a single file - and yes, it's pretty limited.

The change and rename events correspond directly to the UV_CHANGE and UV_RENAME constants. Speaking with my libuv maintainer hat on, I don't foresee us adding a UV_ATTRIB flag because then we have to start tracking metadata and end up implementing half of uv_fs_poll_t / fs.watchFile.

I'll take the liberty of closing this because things are working as expected. The caveats about fs.watch's platform differences are mentioned in the documentation. It was specifically added as a performance optimization over fs.watchFile for people who are prepared to deal with its non-uniform behavior.

The workaround to get the desired behavior is indeed as you say: either stat the file after an event or switch to fs.watchFile, which does that automatically for you.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Sep 28, 2023
@llllvvuu
Copy link
Author

llllvvuu commented Sep 28, 2023

I just sampled Go's fsnotify/fsnotify and Rust's notify-rs/notify, and it seems like they both have this issue as well, that can only be got around by:

  1. allowing the user to pick the backend, i.e. the following fixes the issue in Rust:
- notify = { version = "6.1.1", features = ["macos_kqueue"] }
+ notify = { version = "6.1.1", features = ["macos_fsevent"] }
  1. exposing the event kind to the user (converted to a platform-independent enum)

It sounds like neither are going to happen in Node.js unfortunately (and is really an issue with kqueue so fair enough) so I guess I will have to begin the work of sending out PRs in downstream repros

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants