Allow multiple writes to a Stream in a single Cmd.batch - #132
Open
dkoontz wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow multiple writes in a single
Cmd.batchwithout encounteringLockederrorsDepends on #133
Problem
When two or more write/enqueue/close operations on the same
WritableStreamwere issued in the same tick (e.g. viaCmd.batchorTask.concurrent), the second would fail with a spuriousLockederror. Each operation checkedstream.lockedand calledgetWriter()synchronously, but only released the lock inside a later microtask (writer.ready.then(...)). The secondgetWriter()therefore ran against a still-held lock. This made it unsafe to do things like write in multiple places to stdout or to a log file stream.Fix
Serialize all writer-acquiring operations on a given stream through a per-stream FIFO promise chain, so each operation only acquires its writer after the previous operation's queued promise has either fulfilled or rejected.
src/Gren/Kernel/Stream.js_Stream_writeChains(WeakMap): one FIFO chain perWritableStream. Keyed by the stream object, so independent streams don't block each other and dead streams are GC'd._Stream_runChained(stream, work): appendsworkto the chain and storesrun.then(noop, noop)back into the WeakMap so the next operation'sprevis always fulfilled. Without these recovery handlers, a single rejectedworkwould propagate down the chain and cause every subsequentprev.then(work)to skip itsworkentirely (a fulfillment handler never fires on a rejected promise), permanently breaking the stream after one failure._Stream_rejectLocked()+_Stream_reportRun(...): thestream.lockedcheck now runs inside the queuedwork(not synchronously at the top), so it reflects the real lock state at execution time. A{ __grenStreamLocked: true }sentinel is mapped back toStream.Locked; everything else maps toStream.Cancelled._Stream_toUint8Array: factored out the repeatedDataView→Uint8Arrayconversion._Stream_write,_Stream_enqueue, and_Stream_closeWritablewere all rewritten on top of these helpers.writestill awaits thewriter.write()promise (backpressure-aware);enqueuestill drops it (fire-and-forget into the buffer) respecting the semantics specified insrc/Stream.gren.Prerequisite commit (
cc140eb)_Stream_closeWritablewas fixed to awaitwriter.close()before releasing the lock and to surface close errors asCancelled, instead of firing-and-forgetting. This is required for the serialization work to behave correctly and is included in the PR.Tests
New
integration_tests/src/Test/Stream.gren(registered inMain.gren) covers:Cancelled(close semantics unchanged).Locked— writes against a writable held by an activepipeThroughorpipeTostill correctly reportLocked, confirming the FIFO chain didn't swallow real lock conflicts.Risk / Notes
enqueuecontinues to resolve before the sink has processed the chunk (per its doc), so write errors on the droppedwriter.write()promise still won't flow through_Stream_reportRunbut that's pre-existing documented behavior.