WinFSP: Errors from unlink don't seem to be passed back to userspace

ncw commented 7 years ago

Using

rclone -vv cmount c:\Users\Dev Q:

I did this with python

>>> f = open("Q:/text.txt", "w")
>>> f.write("hello")
>>> f.close()
>>> import os
>>> os.unlink("Q:/text.txt")
>>> # note no error here

However I see this in the rclone log

2017/05/08 19:31:58 DEBUG : /text.txt: Unlink()
2017/05/08 19:31:58 DEBUG : text.txt: Dir.Remove
2017/05/08 19:31:58 ERROR : text.txt: Dir.Remove file error: remove \\?\c:\Users
\Dev\text.txt: The process cannot access the file because it is being used by an
other process.
2017/05/08 19:31:58 ERROR : IO error: remove \\?\c:\Users\Dev\text.txt: The proc
ess cannot access the file because it is being used by another process.
2017/05/08 19:31:58 DEBUG : /text.txt: Unlink() returning -5
2017/05/08 19:31:58 DEBUG : text.txt: ReadFileHandle.Flush
2017/05/08 19:31:58 DEBUG : text.txt: ReadFileHandle.Flush OK
2017/05/08 19:31:58 DEBUG : text.txt: ReadFileHandle.Release closing
2017/05/08 19:31:58 DEBUG : text.txt: ReadFileHandle.Release OK

And the underlying file hasn't been deleted

C:\Users\Dev>dir
 Volume in drive C has no label.
 Volume Serial Number is 5CD1-D250

 Directory of C:\Users\Dev

08/05/2017  19:31    <DIR>          .
08/05/2017  19:31    <DIR>          ..
[snip]
08/05/2017  19:31                 5 text.txt
C:\Users\Dev>

The in use error is caused by the delayed Flush/Close which comes some time after the close in userspace. This I think is a fuse oddity.

This error gets translated to -5 EIO, however the error from Unlink didn't get reported back to userspace - I would have expected the os.unlink to raise an exception.

This is the definition of Unlink I've been using for these tests

// Unlink removes a file.
func (fsys *FS) Unlink(filePath string) (errc int) {
    fs.Debugf(filePath, "Unlink()")
    defer func() {
        fs.Debugf(filePath, "Unlink() returning %d", errc)
    }()
    leaf, parentDir, errc := fsys.lookupParentDir(filePath)
    if errc != 0 {
        return errc
    }
    return translateError(parentDir.Remove(leaf))
}

Any ideas?

billziss-gh commented 7 years ago

Ah, welcome to the wonderful world of Windows file systems :) Where your file system is almost, but not quite, deterministic just because it runs on Windows.

Unlink on UNIX

Unlink (and rmdir) on UNIX is a rather simple and self-contained API (even at the file system level). Its purpose is to remove a directory entry and also the associated inode if there are no links left. If the file is open the inode remains until the file is closed; file systems will often implement this by keeping the file around in an internal list, in some file systems I have even implemented this by moving the file to a hidden (invisible to the user) directory.

DeleteFile on Windows

DeleteFile (and RemoveDirectory) on Windows is a different story. There is no simple and self-contained operation for deleting files at the file system level. Instead deleting files on Windows involves the following:

Open the file with DELETE access. This operation can report errors. [IRP_MJ_CREATE]
Set the "disposition" flag on the file. This operation can report errors. For example, it may report "directory not empty". [IRP_MJ_SET_INFORMATION / FileDispositionInformation]
"Cleanup" the file. This is called when the handle to a file is closed. If the file is being deleted and this was the last handle being closed, go ahead and "delete" the file by removing its directory entry, etc. This operation cannot report errors. [IRP_MJ_CLEANUP]
Close the file. Called when the last reference to the file goes away. [Even when user mode has closed all handles to a file, kernel mode may keep the file around by a "reference"; this is used, for example, by the Lazy Writer that buffers writes to give the illusion of increased write performance.][IRP_MJ_CLOSE]

[There is an alternative deletion protocol used in later versions of Windows for files (not directories), but to keep things simpler I will not include it in this discussion. If you are interested it involves FILE_FLAG_DELETE_ON_CLOSE.]

For the purposes of our discussion the important thing to note is that Cleanup is the place to actually delete a file, but it cannot report an error! Any file system checks and associated errors must be caught during the Open and "Set disposition" stages. Once the file system checks are complete the file system is considered committed to delete the file (but it can fail and in fact it does even on NTFS).

[There is actually a worse problem with deleting files on Windows, which is really why I consider the Windows file system not fully deterministic. It is possible to get a SUCCESS response from an API such as DeleteFile, while the file persists indefinitely. This can happen when multiple processes access the same file.]

WinFsp-FUSE and deleting files/directories

WinFsp-FUSE implements file/directory deletion by following the Windows protocol:

Open checks that the file exists and that the requesting user has DELETE access. Then it calls FUSE open.
CanDelete ("Set disposition") checks whether a directory is empty (by issuing a readdir). No other checks are made for files.
Cleanup (under the right circumstances) actually calls unlink or rmdir on the FUSE file system. If these calls report an error, the error is lost because Cleanup has no way of communicating errors!
Close closes the file by calling FUSE close.

"Being used by another process"

Let's explore now why we are getting the message:

2017/05/08 19:31:58 ERROR : IO error: remove \\?\c:\Users\Dev\text.txt: The proc
ess cannot access the file because it is being used by another process.

The most likely reason for this problem is that the file \\?\c:\Users\Dev\text.txt is being opened without full sharing. I recommend opening your files with FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE if you want to be able to delete them while they are open. Unfortunately looking at the Go source code files are opened with FILE_SHARE_READ | FILE_SHARE_WRITE only.

ncw commented 7 years ago

@ncw wrote:

The most likely reason for this problem is that the file \?\c:\Users\Dev\text.txt is being opened without full sharing. I recommend opening your files with FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE if you want to be able to delete them while they are open. Unfortunately looking at the Go source code files are opened with FILE_SHARE_READ | FILE_SHARE_WRITE only.

Yes, that is exactly the conclusion I came to. In the past it has been suggested that this default change but the consenus was against it.

In the mean time people (like me) will have to keep copying it from the standard library and patching it.

@billziss-gh wrote:

I was rather surprised about how this issue was handled. "We are not fixing this, because we do not quite understand what FILE_SHARE_* does. So we are going to stick with the current share mode."

I actually agree with the OP that the most POSIX-like way of opening files on Windows is by using FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE. IMO it should have been default on Windows and it is a legacy of DOS that it is not. [Actually Windows should just have a true POSIX interface to the file system, but let's not go there.]

https://blogs.msdn.microsoft.com/larryosterman/2004/05/13/why-is-it-file_share_read-and-file_share_write-anyway/

billziss-gh commented 7 years ago

Sorry I am an idi^H^H^Hcareless. I ended up editing your message rather than posting my own. I do not know if there is a way to undo this on github.

billziss-gh commented 7 years ago

As there does not seem to be any actionable item for cgofuse on this issue, I am closing it. Please reopen if you disagree.

winfsp / cgofuse