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 <me@jedevc.com>
			
			
This commit is contained in:
		| @@ -288,7 +288,6 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str | |||||||
| 		resp, err := req.doWithRetries(ctx, nil) | 		resp, err := req.doWithRetries(ctx, nil) | ||||||
| 		if err != nil { | 		if err != nil { | ||||||
| 			pushw.setError(err) | 			pushw.setError(err) | ||||||
| 			pushw.Close() |  | ||||||
| 			return | 			return | ||||||
| 		} | 		} | ||||||
|  |  | ||||||
| @@ -298,7 +297,6 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str | |||||||
| 			err := remoteserrors.NewUnexpectedStatusErr(resp) | 			err := remoteserrors.NewUnexpectedStatusErr(resp) | ||||||
| 			log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response") | 			log.G(ctx).WithField("resp", resp).WithField("body", string(err.(remoteserrors.ErrUnexpectedStatus).Body)).Debug("unexpected response") | ||||||
| 			pushw.setError(err) | 			pushw.setError(err) | ||||||
| 			pushw.Close() |  | ||||||
| 		} | 		} | ||||||
| 		pushw.setResponse(resp) | 		pushw.setResponse(resp) | ||||||
| 	}() | 	}() | ||||||
| @@ -431,6 +429,7 @@ func (pw *pushWriter) Write(p []byte) (n int, err error) { | |||||||
| 		select { | 		select { | ||||||
| 		case <-pw.done: | 		case <-pw.done: | ||||||
| 		case err = <-pw.errC: | 		case err = <-pw.errC: | ||||||
|  | 			pw.Close() | ||||||
| 		case p := <-pw.pipeC: | 		case p := <-pw.pipeC: | ||||||
| 			return 0, pw.replacePipe(p) | 			return 0, pw.replacePipe(p) | ||||||
| 		} | 		} | ||||||
| @@ -488,6 +487,7 @@ func (pw *pushWriter) Commit(ctx context.Context, size int64, expected digest.Di | |||||||
| 	case <-pw.done: | 	case <-pw.done: | ||||||
| 		return io.ErrClosedPipe | 		return io.ErrClosedPipe | ||||||
| 	case err := <-pw.errC: | 	case err := <-pw.errC: | ||||||
|  | 		pw.Close() | ||||||
| 		return err | 		return err | ||||||
| 	case resp = <-pw.respC: | 	case resp = <-pw.respC: | ||||||
| 		defer resp.Body.Close() | 		defer resp.Body.Close() | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Justin Chadwell
					Justin Chadwell