Skip to content

Commit

Permalink
Fix invalid argument error on push
Browse files Browse the repository at this point in the history
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 <agoldste@redhat.com>
  • Loading branch information
Andy Goldstein committed Dec 3, 2014
1 parent 70c4b4e commit 48ec176
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 4 deletions.
21 changes: 17 additions & 4 deletions pkg/archive/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions pkg/archive/archive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os/exec"
"path"
"path/filepath"
"strings"
"syscall"
"testing"
"time"
Expand Down Expand Up @@ -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)
}
}
}

0 comments on commit 48ec176

Please sign in to comment.