copy: prevent potential deadlock if close before fully written

jedevc · jedevc · commit 52a1402df64c · 2024-03-04T12:59:45.000Z
We also need an additional check to avoid setting both the error and
response which can create a race where they can arrive in the receiving
thread in either order.

If we hit an error, we don't need to send the response.

&gt; There is a condition where the registry (unexpectedly, not to spec)
&gt; returns 201 or 204 on the put before the body is fully written. I would
&gt; expect that the http library would issue close and could fall into a
&gt; deadlock here. We could just read respC and call setResponse. In that
&gt; case ErrClosedPipe would get returned and Commit shouldn't be called
&gt; anyway.

Signed-off-by: Justin Chadwell &lt;me@jedevc.com&gt;
diff --git a/remotes/docker/pusher.go b/remotes/docker/pusher.go
@@ -301,6 +301,7 @@ 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)
+			return
 		}
 		pushw.setResponse(resp)
 	}()
@@ -436,6 +437,8 @@ func (pw *pushWriter) Write(p []byte) (n int, err error) {
 			pw.Close()
 		case p := <-pw.pipeC:
 			return 0, pw.replacePipe(p)
+		case resp := <-pw.respC:
+			pw.setResponse(resp)
 		}
 	}
 	status.Offset += int64(n)

Original file line number	Diff line number	Diff line change
`@@ -301,6 +301,7 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str`
`301`	`301`	`err := remoteserrors.NewUnexpectedStatusErr(resp)`
`302`	`302`	`log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response")`
`303`	`303`	`pushw.setError(err)`
	`304`	`+ return`
`304`	`305`	`}`
`305`	`306`	`pushw.setResponse(resp)`
`306`	`307`	`}()`
`@@ -436,6 +437,8 @@ func (pw *pushWriter) Write(p []byte) (n int, err error) {`
`436`	`437`	`pw.Close()`
`437`	`438`	`case p := <-pw.pipeC:`
`438`	`439`	`return 0, pw.replacePipe(p)`
	`440`	`+ case resp := <-pw.respC:`
	`441`	`+ pw.setResponse(resp)`
`439`	`442`	`}`
`440`	`443`	`}`
`441`	`444`	`status.Offset += int64(n)`