From 930ae3dbcb11091955ca936c280d13f24494b245 Mon Sep 17 00:00:00 2001 From: Aaron Lehmann Date: Wed, 30 Mar 2016 19:26:12 -0700 Subject: [PATCH] Pull: only close temporary file once Close could be called twice on a temporary download file, which could have bad side effects. This fixes the problem by setting to ld.tmpFile to nil when the download completes sucessfully. Then the call to ld.Close will have no effect, and only the download manager will close the temporary file when it's done extracting the layer from it. ld.Close will be responsible for closing the file if we hit the retry limit and there is still a partial download present. Fixes #21675 Signed-off-by: Aaron Lehmann --- distribution/pull_v1.go | 15 ++++++++++++++- distribution/pull_v2.go | 14 +++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/distribution/pull_v1.go b/distribution/pull_v1.go index a8ae62491527f84536f075c0c18616ae9848aaa5..86fad2ef7a90aba64129820309b775383eacce08 100644 --- a/distribution/pull_v1.go +++ b/distribution/pull_v1.go @@ -330,7 +330,20 @@ func (ld *v1LayerDescriptor) Download(ctx context.Context, progressOutput progre logrus.Debugf("Downloaded %s to tempfile %s", ld.ID(), ld.tmpFile.Name()) ld.tmpFile.Seek(0, 0) - return ld.tmpFile, ld.layerSize, nil + + // hand off the temporary file to the download manager, so it will only + // be closed once + tmpFile := ld.tmpFile + ld.tmpFile = nil + + return ioutils.NewReadCloserWrapper(tmpFile, func() error { + tmpFile.Close() + err := os.RemoveAll(tmpFile.Name()) + if err != nil { + logrus.Errorf("Failed to remove temp file: %s", tmpFile.Name()) + } + return err + }), ld.layerSize, nil } func (ld *v1LayerDescriptor) Close() { diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go index 596d1c13216ab2f676efec0e2ceca30b15d69356..ecddc170b796a5d11228d523efe0e1645ccb47fb 100644 --- a/distribution/pull_v2.go +++ b/distribution/pull_v2.go @@ -278,7 +278,19 @@ func (ld *v2LayerDescriptor) Download(ctx context.Context, progressOutput progre ld.verifier = nil return nil, 0, xfer.DoNotRetry{Err: err} } - return tmpFile, size, nil + + // hand off the temporary file to the download manager, so it will only + // be closed once + ld.tmpFile = nil + + return ioutils.NewReadCloserWrapper(tmpFile, func() error { + tmpFile.Close() + err := os.RemoveAll(tmpFile.Name()) + if err != nil { + logrus.Errorf("Failed to remove temp file: %s", tmpFile.Name()) + } + return err + }), size, nil } func (ld *v2LayerDescriptor) Close() {