Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

fs.watch fails to detect changes to file replaced by rename event #2062

Closed
TrevorBurnham opened this issue Nov 9, 2011 · 20 comments
Closed
Labels

Comments

@TrevorBurnham
Copy link

If I fs.watch a file and then save a change to it using the Coda text editor, it first detect a 'rename' event, then no further events. Coda appears to be very unusual in this respect; I haven't seen this behavior with any other Mac text editor. Maybe this is a fundamental problem with file change event notifications under Darwin; I'm not sure.

@bnoordhuis
Copy link
Member

I suspect that Coda first writes to a temp file, then does an atomic rename(), hence you get only one event.

@TrevorBurnham
Copy link
Author

Confirmed, this test case gets the same behavior:

$ touch file1
$ touch file2

node> fs.watch('file1', function(event){console.log(event);});

$ mv file2 file1
rename
$ echo 'str' > file1

No response to further changes to file1.

@TrevorBurnham
Copy link
Author

So should I be spawning a new fs.watch whenever I detect a rename event? If so, why doesn't Node do this for me automatically?

@TrevorBurnham
Copy link
Author

Over at CoffeeScript, we're doing this for our compilation utility: https://github.com/TrevorBurnham/coffee-script/blob/d30aa6d621cdd6c5af4eb8514ba30b3ddc97d567/src/command.coffee#L174-186

Is it the Node team's intention to make fs.watch a more low-level API than fs.watchFile, requiring fs.stat calls and a response to the "rename" event for even the most common use case?

@TrevorBurnham
Copy link
Author

Also, it appears that "rename" can mean either "The target file was moved elsewhere" or "Another file replaced the target file." Is that correct? Is that how kqueue reports such things?

@bnoordhuis
Copy link
Member

So should I be spawning a new fs.watch whenever I detect a rename event?

Yes.

If so, why doesn't Node do this for me automatically?

Maybe it should. The kqueue implementation works different from the inotify implementation right now because inotify monitors the directory entry while kqueue monitors the inode.

@TrevorBurnham
Copy link
Author

It turns out there's a problem with re-watching a file whenever a rename event is detected: A rename event can mean either "a different file was renamed on top of this one" (in which case you want to re-watch it) or "this file was renamed elsewhere" (in which case attempting to re-watch it will cause an error). There's no way to distinguish between the two, at least on the Mac.

Worse, fs.watch 'nonexistentfile' currently causes the process to crash, rather than throwing an exception—at least on the Mac. Try it for yourself:

node> > fs = require('fs'); try {fs.watch('nonexistentfile', function(){});} catch(e){}
Assertion failed: (0), function uv_close, file src/unix/core.c, line 149.
Abort trap: 6

As a result, you always have to fs.stat before fs.watch-ing a file. And even then, surely there's a small chance that the file will disappear between the fs.stat and the fs.watch?

@TrevorBurnham
Copy link
Author

surely there's a small chance that the file will disappear between the fs.stat and the fs.watch?

Confirmed, test case:

var fs = require('fs');

for (var i = 1; i <= 100; i++) {
  fs.writeFile('foo.txt', 'foo');
  fs.unlink('foo.txt');
  fs.stat('foo.txt', function(err) {
    if (err) return;
    fs.watch('foo.txt', function() {});
  });
}

@bnoordhuis
Copy link
Member

Worse, fs.watch 'nonexistentfile' currently causes the process to crash, rather than throwing an exception

I'd rather you had reported that as a separate issue but it's fixed in 8dd4fcb.

@TrevorBurnham
Copy link
Author

Thanks for the patch! I'll try to create finer-grained issues in the future.

On Nov 10, 2011, at 1:28 PM, Ben Noordhuisreply@reply.github.com wrote:

Worse, fs.watch 'nonexistentfile' currently causes the process to crash, rather than throwing an exception

I'd rather you had reported that as a separate issue but it's fixed in 8dd4fcb.


Reply to this email directly or view it on GitHub:
#2062 (comment)

@TrevorBurnham
Copy link
Author

Looking at this issue now, I understand that this is the expected behavior: The file has been unlinked, and a new file put in its place; from fs.watch's perspective, its target no longer exists.

But, it's very unfortunate that a rename event can mean either "This file has moved away and is still being watched" or "This file has been unlinked" (and, of course, will not generate any more events). Relatedly, if watched the file has moved, there seems to be no way to find out where it's moved to.

So, @bnoordhuis, would it be possible to add a watchedLocation method or property to the FSWatcher instance (which would produce null if the target is no longer being watched)? That would be a huge win.

@bnoordhuis
Copy link
Member

The problem is that kqueue is file descriptor (inode) based. As long as the file descriptor is open, the old file isn't really unlinked, it's just not visible in the file system any more. You will even get events for the old file if another process also has an open fd and writes to it.

I'm considering making it more in line with how linux and windows behave by reopening the file descriptor after every event but that is:

  1. potentially slow
  2. race-y - you miss events that happen between the close() and open() calls

@TrevorBurnham
Copy link
Author

race-y - you miss events that happen between the close() and open() calls

It's already the case that some events are slipping through the cracks (if I do 10 touches in 5 milliseconds, I get about 6-8 change events), so I wouldn't see this as a significant problem.

I would see it as a problem, though, if the behavior became less internally consistent. E.g., if a file is overwritten via an mv (or an rm immediately followed by an mv), would reopening the file descriptor mean that the new file could become the new fs.watch target, depending on the timing?

@bnoordhuis
Copy link
Member

It's already the case that some events are slipping through the cracks (if I do 10 touches in 5 milliseconds, I get about 6-8 change events), so I wouldn't see this as a significant problem.

That's expected (and sensible) behaviour: if there are several events that amount to the same thing then the kernel collapses them into a single event. It's not kqueue specific, inotify and ReadDirectoryChangesW work the same.

Example: say that there have been three mtime updates since the last poll. The kernel will simply discard the first two because only the last one is relevant.

I would see it as a problem, though, if the behavior became less internally consistent. E.g., if a file is overwritten via an mv (or an rm immediately followed by an mv), would reopening the file descriptor mean that the new file could become the new fs.watch target, depending on the timing?

Yes, no, maybe. The rm + mv case is particularly troublesome. There is a time window where the file simply doesn't exist so rearming the watcher may fail with a 'file not found' error.

@ricardobeat
Copy link

It seems possible to detect path changes with FSEvents (related #2023)

Is there any way to get a filename from a file descriptor?

@michaelficarra
Copy link

@ricardobeat: A file descriptor may have more than one filename associated with it.

edit: wording

@TrevorBurnham
Copy link
Author

And Node uses kqueue under OS X, not FSEvents.

On Nov 19, 2011, at 6:03 PM, Michael Ficarrareply@reply.github.com wrote:

@ricardobeat: A file descriptor may not have only a single filename associated with it.


Reply to this email directly or view it on GitHub:
#2062 (comment)

@ricardobeat
Copy link

And Node uses kqueue under OS X, not FSEvents.

That's why I mentioned issue #2023.

@michaelficarra: OSX seems to support it with fcntl(fd, F_GETPATH, ..), as suggested in Apple's documentation, I wondered if this was available on linux. With that you could open an fd and get the new path after a rename (yeah, ugly).

node-inotify exposes file moves by correlating IN_MOVED_FROM + IN_MOVED_TO, but it probably won't catch moves up on the directory tree.

@ericedem
Copy link

@viatropos did you intend to reopen this issue? This issue seems to be resolved.

@jasnell
Copy link
Member

jasnell commented May 15, 2015

Closing. Issue appears to have been resolved. No information as to why it was reopened. Can reopen if new information is received.

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

No branches or pull requests

6 participants