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

Very weird fs.unlink() behavior with file that is locked by another process #7164

Closed
bpasero opened this issue Feb 21, 2014 · 14 comments
Closed
Labels

Comments

@bpasero
Copy link

bpasero commented Feb 21, 2014

Hi,

I am hunting down an issue which I believe is wrong node.js behavior that reproduces in latst node 0.10.26. I am on Windows and I wrote a small node program that simply tries to delete a file that is used by another process (in this case another node process). The delete actually succeeds for the file although the file is in use. However, a call to fs.readDir() shows me the file still exists. Finally, trying to stat it now results in an EPERM error (Error: EPERM, operation not permitted). I am then in a state where I have to kill the process that locks the file to actually remove it from disk.

The real issue here is that the delete suceeded in the first place and then brought the file system into a state where readDir() shows the file, but stat() is no longer able to stat it. The right behavior (and btw this is how windows behaves if I try to use the explorer) is to not allow the delete in the first place.

I have created a GitHub repo to reproduce: https://github.com/bpasero/nodejsbug.git

To reproduce:

  • clone the repo
  • in one terminal type "node server.js"
  • in another terminal type "node fsdelete.js"
  • in the same terminal type "node fsreaddir.js" and see the file still shows up
  • now try "node fsstat.js" and see that you cannot stat the file

Thanks for looking into this,
Ben

@indutny
Copy link
Member

indutny commented Feb 21, 2014

@orangemocha do you think this could be a node.js issue? Seems like a weird OS behavior to me.

@obastemur
Copy link

@bpasero fs.unlinkSync markes the file for deletion on Windows. If that file in use, it will be deleted as soon as all the handles to that file are closed. If this is not something intended, it can be changed.

Right now it creates the handle with the below flags; (libuv - win - fs.c)

handle = CreateFileW(pathw, FILE_READ_ATTRIBUTES | DELETE, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS, NULL);

if the intention is to return an exception when that file in use, the shared flags needs a replacement.

@bpasero
Copy link
Author

bpasero commented Feb 21, 2014

+1 for a way to unlink with error if file is in use.

this issue also exists for the async unlink().

is there a short term work around I can apply? would a stat on the locked file tell me the file is locked? I could then through the error myself.

@obastemur
Copy link

is there a short term work around I can apply? would a stat on the locked file tell me the file is locked? I could then through the error myself.

not with node. Because it creates the file handles with shared flags whenever you access it from another node process. Actually this behavior is not a problematic one since it opens a way for many kind of applications. IMO it can be changed for 'unlink' only since there is no sense to keep a shared handle to a file expected to be deleted.

Actually the fix is very easy;
Replace that with
handle = CreateFileW(pathw, FILE_READ_ATTRIBUTES | DELETE, NULL, NULL, OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS, NULL);

and it should work as expected.

@bpasero
Copy link
Author

bpasero commented Feb 21, 2014

I see, under these circumstances I will have to wait for a fix in node for unlink(). I also have to see how this behaves when calling fs.rename() on the used file or parent folder, it might have similar behavior.

@sam-github
Copy link

Existing behaviour is intended, and is what I'd expect, because its similar to unix. On unix, if you unlink a file, it still exists for all processes that currently have it open. Its not visible in directory tree, but some differences between windows and unix are bound to leak out. Locking in particular is so different libuv didn't even bother trying to abstract. Its not even consistent between unixen.

As far as I can see, with your "easy" fix, you just get a different kind of cross-platform incompatibility, that will surprise people other than yourself, a bit of a lose-lose situation.

fs.unlink() should probably just be documented! The current docs say "Asynchronous unlink(2).", which is clearly not a useful description of what it does on Windows, which has no unlink(2).

@obastemur
Copy link

That easy fix is a suggestion for @bpasero . Surely, you are right for the behavior consistency. On the other hand, as you already mentioned, there is no unlink on windows and Node's only 'delete file' command just doesn't suit there 100%

@indutny
Copy link
Member

indutny commented Feb 21, 2014

I'll gratefully accept PR that will improve documentation on this topic, thanks for sharing your thoughts. ;)

@obastemur
Copy link

Actually between windows and unix, the only difference is that on unix a new process can create a file with a same name and continue operating on it.

Based on this difference, one another suggestion here is could be; (not a cool hack but solves the problem when it's badly needed) @bpasero ?

if(fs.existsSync(file))
    fs.renameSync('file', aUniqueNameHere);  // It's not going to affect the current open handles
if(fs.existsSync(aUniqueNameHere))
    fs.unlinkSync(aUniqueNameHere);

@trevnorris trevnorris added the fs label Feb 22, 2014
@indutny
Copy link
Member

indutny commented Feb 22, 2014

Seems like this discussion has crossed the border and is not a bug report anymore, closing it. If you wish - you could submit a documentation fix.

@indutny indutny closed this as completed Feb 22, 2014
@bpasero
Copy link
Author

bpasero commented Feb 23, 2014

The thing that still worries me is that unlink() behaves like this but rename() does not. When I try to rename() a file that is locked, I do get the EPERM error that I would expect from unlink(). I think either unlink() should give up with EPERM error like rename() or rename() should work even if a file is locked. Having this inconsistency between these 2 methods seems wrong.

@bpasero
Copy link
Author

bpasero commented Feb 23, 2014

Now that I have investigated a bit more, I had to open three new issues that I think are all originating from this inconsistency. Imho if unlink() on a locked file would fail in the first place, all these issues would not be there. But the fact that you can unlink() it causes weird behavior as explained in the bug reports. There might be even more.

@am11
Copy link

am11 commented Nov 5, 2014

[this one is related to fs.rename but close cousin of this issue]

@indutny, with node-sass, when we build binaries (with node-gyp) followed by npm test, after all tests are run and process exists gracefully, if we try to delete the binary (binding.node file built by node-gyp) from Windows explorer, it says: The action can't be completed because the file is open in Evented I/O for V8 JavaScript and naturally fs.rename also throws EPERM, rename.

This part: "because the file is open in Evented I/O for V8 JavaScript ", is there a fix to release the handle quickly after the process (mocha in this case) exited. I have to kill the node.exe process from Task Manager. The same operation works fine on Ubuntu. More at sass/node-sass#509 (comment).

/cc @kevva

@sagarrayudu
Copy link

I ended the process nodejs in windows task manager. it solved the issue.

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

7 participants