Skip to content

Commit 91a50f7

Browse files
committed
copy: check if writer was closed before setting a pipe
If Close is called externally before a request is attempted, then we will accidentally attempt to send to a closed channel, causing a panic. To avoid this, we can check to see if Close has been called, using a done channel. If this channel is ever done, we drop any incoming errors, requests or pipes - we don't need them, since we're done. Signed-off-by: Justin Chadwell <[email protected]>
1 parent 4660f63 commit 91a50f7

File tree

1 file changed

+35
-19
lines changed

1 file changed

+35
-19
lines changed

core/remotes/docker/pusher.go

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -331,10 +331,12 @@ type pushWriter struct {
331331

332332
pipe *io.PipeWriter
333333

334-
pipeC chan *io.PipeWriter
335-
respC chan *http.Response
334+
done chan struct{}
336335
closeOnce sync.Once
337-
errC chan error
336+
337+
pipeC chan *io.PipeWriter
338+
respC chan *http.Response
339+
errC chan error
338340

339341
isManifest bool
340342

@@ -352,19 +354,30 @@ func newPushWriter(db *dockerBase, ref string, expected digest.Digest, tracker S
352354
pipeC: make(chan *io.PipeWriter, 1),
353355
respC: make(chan *http.Response, 1),
354356
errC: make(chan error, 1),
357+
done: make(chan struct{}),
355358
isManifest: isManifest,
356359
}
357360
}
358361

359362
func (pw *pushWriter) setPipe(p *io.PipeWriter) {
360-
pw.pipeC <- p
363+
select {
364+
case <-pw.done:
365+
case pw.pipeC <- p:
366+
}
361367
}
362368

363369
func (pw *pushWriter) setError(err error) {
364-
pw.errC <- err
370+
select {
371+
case <-pw.done:
372+
case pw.errC <- err:
373+
}
365374
}
375+
366376
func (pw *pushWriter) setResponse(resp *http.Response) {
367-
pw.respC <- resp
377+
select {
378+
case <-pw.done:
379+
case pw.respC <- resp:
380+
}
368381
}
369382

370383
func (pw *pushWriter) Write(p []byte) (n int, err error) {
@@ -374,22 +387,26 @@ func (pw *pushWriter) Write(p []byte) (n int, err error) {
374387
}
375388

376389
if pw.pipe == nil {
377-
p, ok := <-pw.pipeC
378-
if !ok {
390+
select {
391+
case <-pw.done:
379392
return 0, io.ErrClosedPipe
393+
case p := <-pw.pipeC:
394+
pw.pipe = p
380395
}
381-
pw.pipe = p
382396
} else {
383397
select {
384-
case p, ok := <-pw.pipeC:
385-
if !ok {
386-
return 0, io.ErrClosedPipe
387-
}
398+
case <-pw.done:
399+
return 0, io.ErrClosedPipe
400+
case p := <-pw.pipeC:
388401
pw.pipe.CloseWithError(content.ErrReset)
389402
pw.pipe = p
390403

391404
// If content has already been written, the bytes
392-
// cannot be written and the caller must reset
405+
// cannot be written again and the caller must reset
406+
status, err := pw.tracker.GetStatus(pw.ref)
407+
if err != nil {
408+
return 0, err
409+
}
393410
status.Offset = 0
394411
status.UpdatedAt = time.Now()
395412
pw.tracker.SetStatus(pw.ref, status)
@@ -418,7 +435,7 @@ func (pw *pushWriter) Close() error {
418435
// Ensure pipeC is closed but handle `Close()` being
419436
// called multiple times without panicking
420437
pw.closeOnce.Do(func() {
421-
close(pw.pipeC)
438+
close(pw.done)
422439
})
423440
if pw.pipe != nil {
424441
status, err := pw.tracker.GetStatus(pw.ref)
@@ -458,17 +475,16 @@ func (pw *pushWriter) Commit(ctx context.Context, size int64, expected digest.Di
458475
// TODO: timeout waiting for response
459476
var resp *http.Response
460477
select {
478+
case <-pw.done:
479+
return io.ErrClosedPipe
461480
case err := <-pw.errC:
462481
return err
463482
case resp = <-pw.respC:
464483
defer resp.Body.Close()
465-
case p, ok := <-pw.pipeC:
484+
case p := <-pw.pipeC:
466485
// check whether the pipe has changed in the commit, because sometimes Write
467486
// can complete successfully, but the pipe may have changed. In that case, the
468487
// content needs to be reset.
469-
if !ok {
470-
return io.ErrClosedPipe
471-
}
472488
pw.pipe.CloseWithError(content.ErrReset)
473489
pw.pipe = p
474490

0 commit comments

Comments
 (0)