From bc7b73b97ace78fd1cf152acabfb51fea254ef5b Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Thu, 22 Aug 2019 11:52:32 -0700 Subject: [PATCH] pkg/download: close files after extracting in tar (#310) * pkg/download: close files after extracting in tar - Close file handle after extracting files on windows in extractTARGZ(). - Also ensure we close when io.Copy fails on extractZIP(). - Log the ignored error from the cleanup of DownloadPath. Signed-off-by: Ahmet Alp Balkan * add src.close() Signed-off-by: Ahmet Alp Balkan --- pkg/download/downloader.go | 15 ++++++++++----- pkg/installation/install.go | 4 +++- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/download/downloader.go b/pkg/download/downloader.go index ab0d767a..6a5424b0 100644 --- a/pkg/download/downloader.go +++ b/pkg/download/downloader.go @@ -72,17 +72,19 @@ func extractZIP(targetDir string, read io.ReaderAt, size int64) error { dst, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, f.Mode()) if err != nil { + src.Close() return errors.Wrap(err, "can't create file in zip destination dir") } + close := func() { + src.Close() + dst.Close() + } if _, err := io.Copy(dst, src); err != nil { + close() return errors.Wrap(err, "can't copy content to zip destination file") } - - // Cleanup the open fd. Don't use defer in case of many files. - // Don't be blocking - src.Close() - dst.Close() + close() } return nil @@ -131,9 +133,12 @@ func extractTARGZ(targetDir string, at io.ReaderAt, size int64) error { if err != nil { return errors.Wrapf(err, "failed to create file %q", path) } + close := func() { f.Close() } if _, err := io.Copy(f, tr); err != nil { + close() return errors.Wrapf(err, "failed to copy %q from tar into file", hdr.Name) } + close() default: return errors.Errorf("unable to handle file type %d for %q in tar", hdr.Typeflag, hdr.Name) } diff --git a/pkg/installation/install.go b/pkg/installation/install.go index 21b8487d..279e309f 100644 --- a/pkg/installation/install.go +++ b/pkg/installation/install.go @@ -101,7 +101,9 @@ func install(op installOperation, opts InstallOpts) error { } defer func() { glog.V(3).Infof("Deleting the download staging directory %s", op.downloadStagingDir) - _ = os.RemoveAll(op.downloadStagingDir) + if err := os.RemoveAll(op.downloadStagingDir); err != nil { + glog.Warningf("failed to clean up download staging directory: %s", err) + } }() if err := downloadAndExtract(op.downloadStagingDir, op.platform.URI, op.platform.Sha256, opts.ArchiveFileOverride); err != nil { return errors.Wrap(err, "failed to download and extract")