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

readdirSync returns deleted folder #4760

Closed
pavelzaruba opened this issue Jan 19, 2016 · 17 comments
Closed

readdirSync returns deleted folder #4760

pavelzaruba opened this issue Jan 19, 2016 · 17 comments
Labels
fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform.

Comments

@pavelzaruba
Copy link

The function readdirSync returns folder that was already deleted.

steps to reproduce:

  1. create empty app/view folders and go to the view folder in Windows Explorer or file manager
  2. run the following script:
var fs = require('fs');
var res = [];

res.push(fs.readdirSync('app/')); // read subfolders - returns ['view'] array
res.push(fs.rmdirSync('app/view/')); // remove 'view' folder
res.push(fs.readdirSync('app/')); // read subfolders - returns ['view'] array instead of empty one

console.log(res);

expected result:

[ [ 'view' ], undefined, [] ]

actual result:

[ [ 'view' ], undefined, [ 'view' ] ]
@ChALkeR ChALkeR added fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform. labels Jan 19, 2016
@bnoordhuis
Copy link
Member

Do you have a virus scanner or other software installed that hooks system calls?

@pavelzaruba
Copy link
Author

No, just Windows 10 with default Windows Defender. Turning it off doesn't affect the result.

@targos
Copy link
Member

targos commented Jan 19, 2016

Do you need to open the directory in Windows Explorer to get this result ?

@pavelzaruba
Copy link
Author

Yes, but it can be any other file manager.

@bnoordhuis
Copy link
Member

Is the file deleted when you close the file manager? If yes, then it's the file manager that keeps the file alive. Not much node can do about that.

@pavelzaruba
Copy link
Author

The file is deleted immediately after calling the rmdirSync. Most of the file managers (including Windows Explorer) watch the file system changes. If they find the current folder deleted they go up to the parent folder. It's a common behavior and it's the same as in my case - the file manager goes up to app folder.

If you run Command prompt and go to the view folder before running the script, you get the exception:

Error: EBUSY, resource busy or locked 'app\view'

So, if something is holding/locking the folder, you get an exception. If not, shouldn't it wait for the file system since the method is synchronous? (the Sync postfix in the name rmdirSync)

@mscdex
Copy link
Contributor

mscdex commented Jan 19, 2016

/cc @nodejs/platform-windows

@denghongcai
Copy link

try to use

fs.fsyncSync()

I can not reproduce it if I use fs.fsyncSync, seems related to libuv and Windows itself.

@jorangreef
Copy link
Contributor

I can reproduce it on Windows 10.

It's not a bug in Node or libuv, these are correctly returning the current state of the filesystem.

It's a race condition that's just part of working with a filesystem on Windows. Filesystems don't always provide much guarantees as to order and timing of filesystem operations etc. and the user needs to be aware of this.

I think this is what's happening, but I might be wrong:

  1. User opens app/view in Explorer.
  2. Explorer keeps a handle open on app/view.
  3. User removes app/view. Because the handle is kept open by Explorer the rmdir succeeds but Windows waits until all handles have been closed before making this change visible in the filesystem.
  4. User reads the directory listing for app and sees that view is still listed.
  5. Explorer notices step 3 and releases the handle a few ms later.
  6. Windows makes the rmdir change visible in the filesystem.
  7. If the user read the directory listing again now, they would see that view is no longer there.

If it's critical for the user to enforce order, persistence etc. then they should just call fs.fsync as @denghongcai suggested. This is nothing to do with Node or libuv, otherwise there would be thousands of issues for all kinds of filesystem quirks (e.g. utimes on an fd or path being reflected by stat calls etc.) where the solutions to some would cause breaks on others.

@jorangreef
Copy link
Contributor

Also, one needs to run the above test in a script, not a repl, so that the readdir call is made as soon after the rmdir call as possible. Even pasting into the repl does not reproduce it.

@pavelzaruba
Copy link
Author

I see that in the same way as @jorangreef.

NodeJs documentation says the method fsyncSync has an argument called fd, nothing more. So I guess the fd is a file descriptor. How can I get the file descriptor of the folder that is going to be deleted by rmdirSync to pass it later to fsyncSync to wait for releasing all the handles?

Does the fsyncSync method actually wait for releasing all the handles of the folder or it just produces 1 ms time loss which causes returning expected result?

@jorangreef
Copy link
Contributor

@pavelzaruba I think you're right. In this case maybe fsync is actually just giving enough delay so as to avoid the race condition. Otherwise, if you wanted to fsync, you would probably need to just fsync the parent directory of the deleted directory. It's possible to fsync a directory on Linux and OS X (by opening it in read-only mode and then calling fsync on this descriptor) but I don't think it's possible on Windows.

@pavelzaruba
Copy link
Author

Probably not. The folder descriptor passed to fsyncSync...

var fd = fs.openSync('app/', 'r');
fs.fsyncSync(fd);

...throws the exception:

Error: EPERM, operation not permitted
    at Error (native)
    at Object.fs.fsyncSync (fs.js:731:18)

@seishun
Copy link
Contributor

seishun commented Mar 30, 2016

I propose to close this due to reasons outlined by @jorangreef. We can't really fix it in Node.js.

@cjihrig
Copy link
Contributor

cjihrig commented Mar 30, 2016

Agreed.

@HaseenaBegumShabudin
Copy link

Hi All,

I have reproduced the same steps and I am getting the expected result as "[ [ 'view' ], undefined, [] ]" in both Windows and MacOS.

Please let me know, is this defect fixed in node?
Please let me know, if am missing anything here....

Thank you

@yegao
Copy link

yegao commented Nov 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

13 participants