Wait for Windows pipe writes to complete before reporting success - #101
Merged
Merged
Conversation
floitsch
marked this pull request as ready for review
September 13, 2026 15:34
floitsch
pushed a commit
to toitlang/toit
that referenced
this pull request
Sep 13, 2026
Closing Windows TCP sockets with an overlapped receive armed can reset the peer and discard unread data. Closing TCP, UDP, pipe, and UART handles also allows pending completions to write into freed `OVERLAPPED` structures. Add `WindowsOverlapped` to track successfully issued operations and cancel/reap them before destroying their handles, events, and resources. Synchronous failures are not waited on, because no operation was started. TCP writes now use nonblocking `send` and `FD_WRITE` readiness. Each primitive call retries `send` directly; no cached write-readiness flag can overwrite an `FD_WRITE` notification that arrives as a blocked send returns. They report only bytes accepted by the transport, handle partial writes and backpressure, and leave no queued `WSASend` for close to cancel. This also corrects the address length passed to `connect` and closes the auxiliary socket event. Waiting for a pending send inside the shared event thread would stall unrelated I/O; nonblocking sends avoid that dependency. See [Microsoft's send semantics](https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-wsasend). Add `pipe.write-result` to return the actual completed byte count, null while pending, or an asynchronous error. The existing write primitive retains its queued-count behavior for package compatibility. [pkg-host#101](toitlang/pkg-host#101) uses the new primitive to suspend the writing task until completion, serialize concurrent writers, and propagate cancellation/errors. **Preventing pipe write-then-close data loss requires that companion package update.** It remains draft until an SDK release includes the new primitive and its minimum SDK requirement can be updated. UART writes and older host packages still report queued writes; closing them may cancel pending data. The gzip test retains file input while the SDK tests use the released host package.
floitsch
force-pushed
the
fix/windows-pipe-write-completion
branch
from
September 14, 2026 15:27
19e5d07 to
1949a5c
Compare
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.
Windows pipe writes currently return their queued byte count before
WriteFilecompletes. A caller that writes and immediately closes can therefore discard the payload; the regression receives 0 of 262,144 bytes with the existing implementation.Wait for
pipe.write-resultthroughResourceState_before returning the completed count. Serialize writers while a completion is outstanding, propagate asynchronous failures, and abort a writer when another task closes the pipe. Apply the same completion handling to each chunk of genericio.Data.Depends on toitlang/toit#3221, which adds the completion primitive.
package.yamlnow requires^2.0.0-alpha.199, targeting the next SDK release. Keep this PR in draft until that release includes the new primitive. Older SDK compilers do not recognize the new primitive.Validation: cross-built the companion Windows SDK and ran all four new scenarios under Wine: byte-array delivery, chunked string-byte-slice delivery, broken-pipe failure, and concurrent close. The delivery regression fails against the original package. The existing
pipe2_testcould not run because this Wine environment has nocatexecutable.