Operations on deleted FileHandles/DirectoryHandles

[jesup]

While the WPT tests appear to test a few things with deleted File/Directory handles (like that values() on a deleted directory returns NotFoundError), there's no spec text that I see to specify what happens to an existing File/DirectoryHandle when it's removeEntry()'d from it's parent. Ditto SyncAccessHandles.
Also: what happens if an async method is still in progress?

[jesup]

Also don't forget we could have handles in both MainThread and in workers to the same object, and delete it from either

[a-sully]

SyncAccessHandles grab an exclusive lock on a file, so removeEntry() currently isn't allowed, since it grabs a shared lock on the file (the same type of lock acquired by createWritable()). This is not currently specified, since the method was around long before we added locking...

removeEntry() probably SHOULD require an exclusive lock (there's a tracking bug in Chrome) but there's currently a WPT asserting that removal with an open FileSystemWritableFileStream succeeds. I think we should change this.

The proposed move() and remove() methods will require grabbing an exclusive lock on the file, so these operations would not be allowed with either an open SyncAccessHandle or FileSystemWritableFileStream

As for the other async operations, isSameEntry() doesn't look at the contents on disk anyways. getFileHandle() and getDirectoryHandle() have a note to "Better specify what possible exceptions this could throw.", and that could reasonably include what to do if the underlying directory has been deleted

[jesup]

removeEntry() does not (in the spec currently) take a shared lock on the object being removed. Whether it takes a lock in Chrome at the moment isn't the question.

We need to decide what sort of API we want here (linux-style, access is still possible even if removed, or windows-style, can't remove if open). What we appear to have now is neither - errors are returned after removal.

Most of this issue was around "what happens if we have a FileSystemFileHandle or FileSystemDirectoryHandle, and some other operation removeEntry()'s them." A FileSystemFileHandle/FileSystemDirectoryHandle don't hold any sort of lock per spec, only FileSystemSyncAccessHandles and FileSystemWritableFileStreams

[jesup]

I'm going to assume we should take an exclusive lock, and if that fails return InvalidModification

[jesup]

Re TPAC discussion about deleting/moving files with handles:

What are FileHandles/DirectoryHandles?

If you argue they are abstract paths, then deleting the underlying file/directory would cause an error if we try to use the handle, and if we were to move the file/directory, we'd also get an error (unless we moved another file into the spot first). (I believe Chrome has implemented this.)

If you argue they are references to objects resolved at creation time, then deleting the underlying file (from the owning directory) will cause an error if we try to use the file. On the other hand, if we move the file/directory, one would expect that the File/Directory handle would still be usable (and if you call resolve() on it you'd get the new path). (Right now Firefox does this; the internal DB id's are stable across rename.)

So, which is it? I should note that the fact that createFileHandle() takes a create: true option (and fails without that if it doesn't exist) implies that it's not purely an abstract path.

Note another option for remove(): if this was unix-posix-like semantics, removing a file with an active reference would not impact the ability to use existing handles to the removed item.

Once we have a final decision on this, we need to adjust all the spec language to specify what happens in these cases.

[a-sully]

I know that this issue has been inactive for a while now (and that's partly on me - it's quite dependent on #59 and I've been meaning to send an update on that issue for a while) but a WPT was recently updated to assert that createWritable() fails if the file has been removed. The question of "can I delete and then re-create a file from the same handle?" has big implications for the open questions on both this issue and #59). It would be nice to have more discussion on these issues before asserting behaviors in the WPTs... (and I acknowledge I'm likely guilty of doing this, too, so please call me out on that going forward)

One of the key motivating use cases for the remove() of self method is to remove a handle selected via showSaveFilePicker(). From that explainer:

It's quite common for a site to obtain a file handle from showSaveFilePicker(), but then decide not to save after all, and want to delete the file.

Now that remove() is available, it will presumably become common for a site to then decide that they actually do want to save a file. If remove() effectively makes a handle unusable, this is a very frustrating developer experience, since it requires showing yet another file picker to save the file. Developers will be incentivized to avoid using this method outside of the OPFS, resulting a poorer user experience (since the alternative is to leave an empty file).

My understanding of the counter-arguments are:

1. A reference-based design is easier to implement if you can assume a handle is unusable after it was removed
2. Deletion + re-creation of directories from the same handle is currently not possible, though that's certainly something we can change

Perhaps we need a more explicit way to re-create a file from a handle other than createWritable()?

[annevk]

cc @whatwg/fs

[a-sully]

To give some other updates on this issue...

I'm going to assume we should take an exclusive lock, and if that fails return InvalidModification

As of https://github.com/web-platform-tests/wpt/pull/35407, removeEntry() takes an exclusive lock on the file (matching the behavior of remove()). Locking errors should always return NoModificationAllowedError, though

Note another option for remove(): if this was unix-posix-like semantics, removing a file with an active reference would not impact the ability to use existing handles to the removed item.

Outside of the OPFS, a multiple sites may have handles corresponding to the same file. This means a site could be prevented from actually deleting a file is another site happens to have a handle, which is confusing and frustrating for developers. Even if we ignore this and only look at the OPFS, I expect developers to expect that remove() deletes the underlying file/directory (which is what #9 specifies)

We could consider adding a flag to remove() to support "unlink" behavior, but I don't think this should be the default

[a-sully]

Some more updates on this issue...

This core question of this issue was addressed by #96, which clarified that a FileSystemHandle maps to a file path - and therefore if the underlying file is removed, the FileSystemHandle itself is not changed.

From my perspective, there are still a few open related questions, which I've filed separate issues to discuss:

1. Do we need a create() self method? See #126
2. What should createWritable() do if the file does not exist? See #125

Closing this issue for now, but please re-open if there is more to discuss!