From b48e1141ebdeb82c3e575d3820a92cbd9250bb28 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Thu, 13 Apr 2023 17:12:23 +0100 Subject: [PATCH] copy: setError should imply Close If sending two messages from goroutine X: a <- 1 b <- 2 And receiving them in goroutine Y: select { case <- a: case <- b: } Either branch of the select can trigger first - so when we call .setError and .Close next to each other, we don't know whether the done channel will close first or the error channel will receive first - so sometimes, we get an incorrect error message. We resolve this by not sending both signals - instead, we can have .setError *imply* .Close, by having the pushWriter call .Close on itself, after receiving an error. Signed-off-by: Justin Chadwell --- core/remotes/docker/pusher.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/remotes/docker/pusher.go b/core/remotes/docker/pusher.go index cca118d39..128d2fd03 100644 --- a/core/remotes/docker/pusher.go +++ b/core/remotes/docker/pusher.go @@ -288,7 +288,6 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str resp, err := req.doWithRetries(ctx, nil) if err != nil { pushw.setError(err) - pushw.Close() return } @@ -298,7 +297,6 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str err := remoteserrors.NewUnexpectedStatusErr(resp) log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response") pushw.setError(err) - pushw.Close() } pushw.setResponse(resp) }() @@ -431,6 +429,7 @@ func (pw *pushWriter) Write(p []byte) (n int, err error) { select { case <-pw.done: case err = <-pw.errC: + pw.Close() case p := <-pw.pipeC: return 0, pw.replacePipe(p) } @@ -488,6 +487,7 @@ func (pw *pushWriter) Commit(ctx context.Context, size int64, expected digest.Di case <-pw.done: return io.ErrClosedPipe case err := <-pw.errC: + pw.Close() return err case resp = <-pw.respC: defer resp.Body.Close()