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

Exposing 'locked' #40

Closed
jesup opened this issue Jul 28, 2022 · 3 comments
Closed

Exposing 'locked' #40

jesup opened this issue Jul 28, 2022 · 3 comments

Comments

@jesup
Copy link
Contributor

jesup commented Jul 28, 2022

The WPT tests assume a property 'locked' is exposed by a WritableFileStream. This does not appear anywhere in the spec, and I don't see any reason for it to be exposed. The solution here is probably to remove references to 'locked' from WPT

@a-sully
Copy link
Collaborator

a-sully commented Jul 29, 2022

The reason for wanting FileSystemWritableFileStream to acquire a lock is so that a SyncAccessHandle cannot be created when there is an open writable to the file, and vice-versa. This behavior is asserted in these WPTs

However we can't require FileSystemWritableFileStreams to acquire an exclusive lock without making a web-exposed change, since locking was added well after the initial API had been launched, so there was nothing preventing sites from opening multiple FileSystemWritableFileStreams to the same file (though I can't find a WPT asserting this behavior). If there are multiple writers, the last writer wins because of the way writes are written to a swap file and only moved to the target file on close(), which is mentioned non-normatively in the spec:

This is typically implemented by writing data to a temporary file, and only replacing the file represented by fileHandle with the temporary file when the writable filestream is closed.

If we don't like the current behavior, we could require FileSystemWritableFileStreams to acquire an exclusive lock or make locking behavior more explicit with something like #19. Regardless, I agree that this should be specified (#18) :)

@mkruisselbrink
Copy link
Collaborator

mkruisselbrink commented Jul 29, 2022

If you indeed meant FileSystemWritableFileStream in the original comment, that has a locked attribute because it inherits from WritableStream (so it indicates if the stream is locked, not if the underlying file is locked).

@jesup
Copy link
Contributor Author

jesup commented Aug 2, 2022

Yes, I meant FileSystemWritableFileStream, and I missed that WritableStream has 'locked' (when someone else committed FileSystemWritableFileStream.webidl to our repo, it was missing the inheritance from WritableStream, so the wpt test failed). I had assumed 'locked' applied to the locking specified in this spec.

@jesup jesup closed this as completed Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants