Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

File changes can be missed if they occur within 1 second #7929

Open
njx opened this issue May 23, 2014 · 12 comments
Open

File changes can be missed if they occur within 1 second #7929

njx opened this issue May 23, 2014 · 12 comments

Comments

@njx
Copy link

njx commented May 23, 2014

On the Mac, if a file is modified multiple times within the same second, we don't always end up with the latest version of the file in open documents in Brackets, because the resolution of timestamps on the Mac is 1s.

I run into this regularly when rebasing, because the process of rebasing multiple commits leads to the same file being rewritten multiple times in quick succession. (It's a little hard to come up with a brief recipe for reproducing this, but I can make one up if it would help.)

It seems like there are two levels at which this could occur:

  1. In the FileSystem layer, we ignore changes if the cached stat's mtime is the same as the file's current mtime.
  2. In FileSyncManager, we ignore changes if the document's timestamp is the same as the file's mtime.

We could fix (1) by actually re-reading the content when we get a change event and seeing if it changed in order to determine whether to dispatch a change event higher up. Obviously that could be a performance hit, but since in most cases a change event means the file actually did change, it seems like this shouldn't be a huge extra impact.

We could fix (2) by making it so that when FileSyncManager.findExternalChanges() is called as the result of a filesystem change event, we pass in a list of the files that were changed. For those files, FSM could ignore the timestamps and always re-read the files.

(Interestingly, when I reproduce this in the rebase scenario, it seems sometimes like (2) fails but not (1) - i.e., the file is not correct in the open document, but if I close and reopen it I get the correct contents. I'm not sure why that would happen, so there might be some other behavior I'm not aware of here.)

@njx njx assigned peterflynn and unassigned peterflynn May 23, 2014
@njx
Copy link
Author

njx commented May 23, 2014

@peterflynn @gruehle Any thoughts?

@gruehle
Copy link
Member

gruehle commented May 23, 2014

We could also add a size check in addition to the mtime check. This would be very cheap since the stat object already has a size field. For (2), this would require remembering the size along with the timestamp.

This would catch more cases, but would still get fooled if there are multiple changes without a size change.

Re-reading the contents could get very expensive on folder or wholesale changes.

@njx
Copy link
Author

njx commented May 24, 2014

@gruehle Yes, but it seems like if we get a specific file change, we probably need to re-read it anyway. I do remember, though, that on the Mac we tend to get directory changes for the parent instead of individual file changes, at least in some cases. It would be good to understand when/how often that happens.

@njx
Copy link
Author

njx commented Jun 2, 2014

Medium priority to @peterflynn

@njx njx added this to the Brackets 1.0 milestone Jul 17, 2014
@njx
Copy link
Author

njx commented Jul 17, 2014

Marking Brackets 1.0

@njx
Copy link
Author

njx commented Oct 27, 2014

Wondering why this was removed from Brackets 1.0. Note that it might be related to #7638.

@dangoor
Copy link
Contributor

dangoor commented Oct 27, 2014

@njx We're still looking into file watcher-related issues, but had to reduce 1.0 scope along the way (something we'll be doing further right now).

@njx
Copy link
Author

njx commented Oct 27, 2014

OK. It seems like some kind of quick fix like making "Refresh File Tree" toss file caches might be a good stopgap.

@dangoor
Copy link
Contributor

dangoor commented Oct 27, 2014

That does seem like a good stopgap... we wouldn't want people to have to do that, but at least doing a Refresh File Tree will then do what people expect. For the most part, people wouldn't need to refresh the file tree.

@peterflynn any opinion on that?

@njx
Copy link
Author

njx commented Oct 27, 2014

One issue is that that's an obvious workaround for when the file tree seems to be out of sync, but it's not obvious that that's what you should do if you notice that the contents of one of your files is out of sync.

@dangoor
Copy link
Contributor

dangoor commented Oct 28, 2014

Agreed. It's not perfect.

@dynamicdan
Copy link

How to recreate.

Write a script (or node app) that watches for file changes (use chokidar). When a change is detected, attempt to overwrite the file with test content.

Brackets will still show the old version and not the overwritten contents due to the quick change.

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

No branches or pull requests

6 participants