server/router/build: replace output buffering workaround with EnableFullDuplex - #52364
Merged
thaJeztah merged 1 commit intoMay 7, 2026
Merged
Conversation
VedantMadane
force-pushed
the
build-remove-output-buffering-workaround
branch
from
April 15, 2026 14:54
639cdd0 to
4952040
Compare
vvoland
reviewed
Apr 16, 2026
|
|
||
| body := r.Body | ||
| var ww io.Writer = w | ||
| if body != nil { |
Contributor
There was a problem hiding this comment.
Should we keep the r.Body != nil check?
Member
There was a problem hiding this comment.
Looks like it may be redundant for server requests; https://pkg.go.dev/net/http#Request.Body
// For client requests, a nil body means the request has no
// body, such as a GET request. The HTTP Client's Transport
// is responsible for calling the Close method.
//
// For server requests, the Request Body is always non-nil
// but will return EOF immediately when no body is present.
// The Server will close the request body. The ServeHTTP
// Handler does not need to.
…ullDuplex The wrapOutputBufferedUntilRequestRead helper was added as a workaround for Go not supporting full-duplex HTTP/1 responses (golang/go#15527, golang/go#22209). Since Go 1.21, the standard library ships http.ResponseController.EnableFullDuplex which tells the server that the handler will interleave reads and writes, making the buffering wrap unnecessary. Replace the workaround with a single EnableFullDuplex call and remove the helper types (wcf, rcNotifier, flusher, nopFlusher) that were only used by the old path. Fixes moby#51209 Signed-off-by: Vedant Madane <vedant.madane@gmail.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
build-remove-output-buffering-workaround
branch
from
May 6, 2026 23:35
4952040 to
70ac493
Compare
Member
|
Did a quick rebase and fixed the linting issue |
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
3 participants
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.
The
wrapOutputBufferedUntilRequestReadhelper and its support types (wcf,rcNotifier,flusher,nopFlusher) were introduced in #37151 to work around Go not supporting full-duplex HTTP/1 responses (golang/go#15527, golang/go#22209). Without the workaround, writing the build progress stream before the request body (the build context tar) was fully consumed could corrupt the connection.Since Go 1.21 (https://go.dev/cl/472636, https://go.dev/cl/501300), the standard library ships
http.ResponseController.EnableFullDuplexwhich is exactly the recommended replacement. Moby already requires Go 1.25, so we can rely on it unconditionally.This PR replaces the entire buffering machinery with a single
EnableFullDuplexcall at the top ofpostBuild, then passesr.Bodyandwthrough directly. Thebufioimport is removed as it was only used by the workaround.Fixes #51209