From 5e3e0f129e5268a1c85a7003c2456c3d0b5ea0a7 Mon Sep 17 00:00:00 2001 From: Andy Goldstein Date: Wed, 3 Dec 2014 15:36:57 -0500 Subject: [PATCH] Fix invalid argument error on push With 32ba6ab from #9261, TempArchive now closes the underlying file and cleans it up as soon as the file's contents have been read. When pushing an image, PushImageLayerRegistry attempts to call Close() on the layer, which is a TempArchive that has already been closed. In this situation, Close() returns an "invalid argument" error. Add a Close method to TempArchive that does a no-op if the underlying file has already been closed. Signed-off-by: Andy Goldstein --- archive/archive.go | 21 +++++++++++++++++---- archive/archive_test.go | 16 ++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/archive/archive.go b/archive/archive.go index 3783e72..ead85be 100644 --- a/archive/archive.go +++ b/archive/archive.go @@ -771,20 +771,33 @@ func NewTempArchive(src Archive, dir string) (*TempArchive, error) { return nil, err } size := st.Size() - return &TempArchive{f, size, 0}, nil + return &TempArchive{File: f, Size: size}, nil } type TempArchive struct { *os.File - Size int64 // Pre-computed from Stat().Size() as a convenience - read int64 + Size int64 // Pre-computed from Stat().Size() as a convenience + read int64 + closed bool +} + +// Close closes the underlying file if it's still open, or does a no-op +// to allow callers to try to close the TempArchive multiple times safely. +func (archive *TempArchive) Close() error { + if archive.closed { + return nil + } + + archive.closed = true + + return archive.File.Close() } func (archive *TempArchive) Read(data []byte) (int, error) { n, err := archive.File.Read(data) archive.read += int64(n) if err != nil || archive.read == archive.Size { - archive.File.Close() + archive.Close() os.Remove(archive.File.Name()) } return n, err diff --git a/archive/archive_test.go b/archive/archive_test.go index 05362a2..fdba6fb 100644 --- a/archive/archive_test.go +++ b/archive/archive_test.go @@ -9,6 +9,7 @@ import ( "os/exec" "path" "path/filepath" + "strings" "syscall" "testing" "time" @@ -607,3 +608,18 @@ func TestUntarInvalidSymlink(t *testing.T) { } } } + +func TestTempArchiveCloseMultipleTimes(t *testing.T) { + reader := ioutil.NopCloser(strings.NewReader("hello")) + tempArchive, err := NewTempArchive(reader, "") + buf := make([]byte, 10) + n, err := tempArchive.Read(buf) + if n != 5 { + t.Fatalf("Expected to read 5 bytes. Read %d instead", n) + } + for i := 0; i < 3; i++ { + if err = tempArchive.Close(); err != nil { + t.Fatalf("i=%d. Unexpected error closing temp archive: %v", i, err) + } + } +}