From f8390e71978a3c60a43559053f373048feb83092 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Mon, 7 Oct 2019 21:12:03 +0900 Subject: [PATCH] Extract a block containing defer in cache_archive.go's Write loop (#32) The deferred executions are responsible for closing files, but will not be executed until the function returns. Therefore, so many files will remain open at the time the old implementation of Write returns, which can cause "too many files open" error. This change fixes the problem by extracting defer into another function which does not contain a loop. --- cache_archive.go | 74 +++++++++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/cache_archive.go b/cache_archive.go index 2e361c3..0892db5 100644 --- a/cache_archive.go +++ b/cache_archive.go @@ -55,51 +55,59 @@ func NewArchive(pth string, compress bool) (*Archive, error) { // Write writes the given files in the cache archive. func (a *Archive) Write(pths []string) error { for _, pth := range pths { - info, err := os.Lstat(pth) - if err != nil { - return fmt.Errorf("failed to lstat(%s), error: %s", pth, err) + if err := a.writeOne(pth); err != nil { + return err } + } - var link string - if info.Mode()&os.ModeSymlink != 0 { - link, err = os.Readlink(pth) - if err != nil { - return fmt.Errorf("failed to read link(%s), error: %s", pth, err) - } - } + return nil +} - header, err := tar.FileInfoHeader(info, link) +func (a *Archive) writeOne(pth string) error { + info, err := os.Lstat(pth) + if err != nil { + return fmt.Errorf("failed to lstat(%s), error: %s", pth, err) + } + + var link string + if info.Mode()&os.ModeSymlink != 0 { + link, err = os.Readlink(pth) if err != nil { - return fmt.Errorf("failed to get tar file header(%s), error: %s", link, err) + return fmt.Errorf("failed to read link(%s), error: %s", pth, err) } + } - header.Name = pth - header.ModTime = info.ModTime() + header, err := tar.FileInfoHeader(info, link) + if err != nil { + return fmt.Errorf("failed to get tar file header(%s), error: %s", link, err) + } - if err := a.tar.WriteHeader(header); err != nil { - return fmt.Errorf("failed to write header(%v), error: %s", header, err) - } + header.Name = pth + header.ModTime = info.ModTime() - // Calling Write on special types like TypeLink, TypeSymlink, TypeChar, TypeBlock, TypeDir, and TypeFifo returns (0, ErrWriteTooLong) regardless of what the Header.Size claims. - if !info.Mode().IsRegular() { - continue - } + if err := a.tar.WriteHeader(header); err != nil { + return fmt.Errorf("failed to write header(%v), error: %s", header, err) + } - file, err := os.Open(pth) - if err != nil { - return fmt.Errorf("failed to open file(%s), error: %s", pth, err) - } + // Calling Write on special types like TypeLink, TypeSymlink, TypeChar, TypeBlock, TypeDir, and TypeFifo returns (0, ErrWriteTooLong) regardless of what the Header.Size claims. + if !info.Mode().IsRegular() { + return nil + } - defer func() { - if err := file.Close(); err != nil { - log.Warnf("Failed to close file (%s): %s", pth, err) - } - }() + file, err := os.Open(pth) + if err != nil { + return fmt.Errorf("failed to open file(%s), error: %s", pth, err) + } - // Write writes to the current file in the tar archive. Write returns the error ErrWriteTooLong if more than Header.Size bytes are written after WriteHeader. - if _, err := io.CopyN(a.tar, file, info.Size()); err != nil && err != io.EOF { - return fmt.Errorf("failed to copy, error: %s, file: %s, size: %d for header: %v", err, file.Name(), info.Size(), header) + defer func() { + if err := file.Close(); err != nil { + log.Warnf("Failed to close file (%s): %s", pth, err) } + }() + + // Write writes to the current file in the tar archive. Write returns the error ErrWriteTooLong if more than Header.Size bytes are written after WriteHeader. + if _, err := io.CopyN(a.tar, file, info.Size()); err != nil && err != io.EOF { + return fmt.Errorf("failed to copy, error: %s, file: %s, size: %d for header: %v", err, file.Name(), info.Size(), header) } return nil