copy: setError should imply Close

jedevc · pull[bot] · commit 9ab0ec6330a3 · 2024-07-30T21:01:51.000Z
If sending two messages from goroutine X:

	a &lt;- 1
	b &lt;- 2

And receiving them in goroutine Y:

	select {
	case &lt;- a:
	case &lt;- 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 &lt;me@jedevc.com&gt;
diff --git 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()
