search

whatwg / fs

File System Standard
https://fs.spec.whatwg.org/
Other
237 stars 20 forks source link

Possible options in FileSystemFileHandleCreateAccessHandleOptions #19

Closed a-sully closed 1 year ago

a-sully commented 2 years ago

Migrated from https://github.com/WICG/file-system-access/issues/326 (see https://github.com/whatwg/fs/issues/2)

@mkruisselbrink

I've been thinking a bit about what options might make sense to be able to pass to the createAccessHandle and createSyncAccessHandle methods, primarily to make sure we won't paint ourselves into a corner if we initially ship only one "flavor" of access handles (i.e. exclusively locked, in-place writable access handles), as currently described in the AccessHandle proposal.

I'm currently thinking there might be three separate axes on which it could make sense to change the behavior of access handles:

File locking behavior (don't lock the file at all, hold a shared (aka read) lock on the file, or hold an exclusive lock on the file) Should the handle operate on the file in-place, or atomically on a swap/temporary version of the file (either starting out blank or created by copying the previously existing file) Should the handle provide read-only access or allow both reading and writing. I'm not sure if all possible combinations of these make sense, but I think quite a few of them probably do. For example:

We would only allow in-place, readwrite access for files in the origin private file system, for the same reasons we don't have an in-place mode for createWritable today (on the other hand, in-place read-only mode should be just fine for arbitrary file handles).

We probably want to disallow non-locking or shared-locking in-place read-write access, as that would enable changes from one frame to be immediately visible in other frames, resulting in potentially high-frequency timers etc. Requiring a page to first obtain a exclusive lock before being able to write in-place would give us the opportunity to make sure this can't be exploited as a super-low-latency communication mechanism.

One question I'm not sure about is if we'd also want to enable a particular AccessHandle to transition between different locking modes. Is there benefit to being able to upgrade a shared lock to an exclusive lock without having to re-open the access handle (or the reverse, drop from exclusive down to shared/no-lock)? My hope would be that we wouldn't need that, but I'm not an expert in how people use file locking.

Another question is what locking a file actually means for files outside the origin private file system. Enforcing the same locking semantics for other access via the File System Access API in the same browser instance would be a start, but probably we would also want to lock the actual files on disk with whatever file locking mechanism the underlying OS supports. Afaik on posix file locks are advisory only, but on windows shared and exclusive file locks do exist. I think we should aim for a best-effort do whatever is typical on the underlying FS, while enforcing well specified semantics for other access using the File System Access API.

Finally, some strawman IDL proposals:

enum FileSystemLockMode { "none", "shared", "exclusive", };
enum FileSystemAccessMode { "read", "readwrite", };
enum FileSystemWriteMode {"atomic", "atomic-from-copy", "in-place"};

dictionary FileSystemFileHandleCreateAccessHandleOptions {
  FileSystemAccessMode access = "readwrite";
  FileSystemWriteMode mode = "atomic";
  FileSystemLockMode lock;
};

I.e. the default would be similar to what createWritable does today, while if you want the current Access Handle proposal's behavior you'd have to pass {mode: "in-place"}. Not entirely sure what the default for lock should be. For in-place readwrite mode "exclusive" is the only valid option, so that should probably be it. On the other hand for read-only access it might make more sense to default to "shared" if no lock mode is provided.

In an initial implementation of the current Access Handle proposal in chrome we would implement this as

enum FileSystemWriteMode {"in-place"};

dictionary FileSystemFileHandleCreateAccessHandleOptions {
  required FileSystemWriteMode mode;
};

which more or less matches the current Access Handle proposal anyway (the only difference with the current proposal being that mode would be required), while this can relatively easily be backwards compatibly extended to cover the larger feature set described above.

@tomayac

Super nit remark: I know where we come from with this, but the below…

enum FileSystemAccessMode { "read", "readwrite", };

…is the odd one out that’s spelled without a dash ‘-‘.

a-sully commented 2 years ago

Relatedly, we should consider adding an autoClose flag to FileSystemCreateWritableOptions as suggested in https://github.com/WICG/file-system-access/issues/236

@mkruisselbrink

Currently you need to explicitly close a FileSystemWritableFileStream for the changes written to it to be flushed to disk. Usually this is a good thing, since it means that a website won't accidentally overwrite a file with a partial version of the file if for whatever reason the website is closed while it is writing to a file. I.e. changes are written to a temporary file and then atomically moved into place.

Sometimes websites to want to be able to write partial data to files though. For example in the case of an audio or video recorder it might be totally reasonable to just want to write however much data could be written, even if the app is closed unexpectedly.

To support that I propose adding a autoClose flag to FileSystemCreateWritableOptions. By default this is false, keeping the existing behavior. But if it is set to true, changes will be written to the target file even if the stream isn't explicitly closed (when the stream gets garbage collected/the page owning it gets closed). A website can still abort the write by calling abort() on the stream, and websites should still call close() to make sure the data gets moved into place as soon as feasible (relying on garbage collection would it make it very unpredictable when the changes show up on disk).

There is some overlap between this and earlier ideas for an "in-place" mode, but in-place mode makes things like safe browsing and other security checks much harder. So with no clear way forward for in-place, this will at least cover some of its use cases.

fivedots commented 2 years ago

After conversations with Safari and the Chrome Storage team, we are changing the approach to providing options to create[Sync]AccessHandle() in the current Chromium implementation, as well as in the explainer. To avoid compatibility issues and leave the design of the options object fully open, the parameter will be temporarily removed.

Once there is consensus, the options can be re-added without much issue, e.g.: by considering the current OPFS behaviour to be the default, and requiring the paremeter when this would not be appropriate (like when creating it in a non-OPFS file system).

annevk commented 2 years ago

That sounds reasonable, but could you keep the Mozilla storage team in the loop as well going forward?

a-sully commented 1 year ago

Closing in favor of #34, where discussions related to this idea are currently taking place