whatwg / fs

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

No explicit file lock release when `write()` throws an exception #153

Open saschanaz opened 9 months ago

saschanaz commented 9 months ago

What is the issue with the File System Standard?

https://fs.spec.whatwg.org/#write-a-chunk throws when getting a non-FileSystemWriteChunkType:

Let input be the result of converting chunk to a FileSystemWriteChunkType. If this throws an exception, then return a promise rejected with that exception.

But the spec does not say whether this exception should release the lock. In fact, the release only happens within:

  1. closeAlgorithm (step 4.2.2.4.1 in https://fs.spec.whatwg.org/#create-a-new-filesystemwritablefilestream)
  2. abortAlgorithm (step 5.1.1 in https://fs.spec.whatwg.org/#create-a-new-filesystemwritablefilestream)
  3. close() method (https://fs.spec.whatwg.org/#dom-filesystemsyncaccesshandle-close)

Errors from writeAlgorithm does not release the lock at all per the spec, as returning a rejected promise is not enough, which leads to a permanent lock as there's no GC-triggered lock release either.

The FS spec probably can get some algorithm to return a promise that waits for the lock release and use it for each exception, but that's sad as the implementations should do extra care for every implementation-specific error conditions. Could be nice if the Streams spec had some async hook to handle the error.

cc @jjjalkanen cc @domenic @mattiasbuelens

mkruisselbrink commented 9 months ago

Afaict when the writeAlgorithm rejects, WritableStreamDefaultControllerProcessWrite (https://streams.spec.whatwg.org/#writable-stream-default-controller-process-write) calls WritableStreamFinishInFlightWriteWithError which calls WritableStreamDealWithRejection, which calls WritableStreamFinishErroring, which ends up calling the abort algorithm?

MattiasBuelens commented 9 months ago

This sounds a lot like https://github.com/whatwg/streams/issues/636.

saschanaz commented 9 months ago

Afaict when the writeAlgorithm rejects, WritableStreamDefaultControllerProcessWrite (https://streams.spec.whatwg.org/#writable-stream-default-controller-process-write) calls WritableStreamFinishInFlightWriteWithError which calls WritableStreamDealWithRejection, which calls WritableStreamFinishErroring, which ends up calling the abort algorithm?

Step 8 of FinishErroring returns early when there's no pending abort request, so not always.