copy: setError should imply Close

jedevc · jedevc · commit 872746386237 · 2024-03-04T12:59:45.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/remotes/docker/pusher.go b/remotes/docker/pusher.go
@@ -292,7 +292,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
 		}
 
@@ -302,7 +301,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)
 	}()
@@ -435,6 +433,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)
 		}
@@ -492,6 +491,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()
