From 14567952b323751858776c7d572e387829b85766 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Tue, 23 Dec 2014 11:03:34 +0000 Subject: [PATCH] google cloud storage: Fix memory leak - fixes #17 This was the same problem as issue #5 (which affected google drive) --- drive/drive.go | 29 ++---------------- fs/seekwrapper.go | 39 ++++++++++++++++++++++++ googlecloudstorage/googlecloudstorage.go | 6 ++-- 3 files changed, 45 insertions(+), 29 deletions(-) create mode 100644 fs/seekwrapper.go diff --git a/drive/drive.go b/drive/drive.go index eb78ed0cc..0a63b5480 100644 --- a/drive/drive.go +++ b/drive/drive.go @@ -21,7 +21,6 @@ import ( "log" "mime" "net/http" - "os" "path" "strings" "sync" @@ -603,30 +602,6 @@ func (f *FsDrive) ListDir() fs.DirChan { return out } -// seekWrapper wraps an io.Reader with a basic Seek for -// code.google.com/p/google-api-go-client/googleapi -// to detect the length (see getReaderSize function) -type seekWrapper struct { - in io.Reader - size int64 -} - -// Read bytes from the object - see io.Reader -func (file *seekWrapper) Read(p []byte) (n int, err error) { - return file.in.Read(p) -} - -// Seek - minimal implementation for Google Drive's length detection -func (file *seekWrapper) Seek(offset int64, whence int) (int64, error) { - switch whence { - case os.SEEK_CUR: - return 0, nil - case os.SEEK_END: - return file.size, nil - } - return 0, nil -} - // Put the object // // This assumes that the object doesn't not already exists - if you @@ -663,7 +638,7 @@ func (f *FsDrive) Put(in io.Reader, remote string, modTime time.Time, size int64 } // Make the API request to upload metadata and file data. - in = &seekWrapper{in: in, size: size} + in = &fs.SeekWrapper{In: in, Size: size} info, err = f.svc.Files.Insert(info).Media(in).Do() if err != nil { return o, fmt.Errorf("Upload failed: %s", err) @@ -872,7 +847,7 @@ func (o *FsObjectDrive) Update(in io.Reader, modTime time.Time, size int64) erro } // Make the API request to upload metadata and file data. - in = &seekWrapper{in: in, size: size} + in = &fs.SeekWrapper{In: in, Size: size} info, err := o.drive.svc.Files.Update(info.Id, info).SetModifiedDate(true).Media(in).Do() if err != nil { return fmt.Errorf("Update failed: %s", err) diff --git a/fs/seekwrapper.go b/fs/seekwrapper.go new file mode 100644 index 000000000..6975bfb31 --- /dev/null +++ b/fs/seekwrapper.go @@ -0,0 +1,39 @@ +package fs + +import ( + "io" + "os" +) + +// SeekWrapper wraps an io.Reader with a basic Seek method which +// returns the Size attribute. +// +// This is used for google.golang.org/api/googleapi/googleapi.go +// to detect the length (see getReaderSize function) +// +// Without this the getReaderSize function reads the entire file into +// memory to find its length. +type SeekWrapper struct { + In io.Reader + Size int64 +} + +// Read bytes from the object - see io.Reader +func (file *SeekWrapper) Read(p []byte) (n int, err error) { + return file.In.Read(p) +} + +// Seek - minimal implementation for Google API length detection +func (file *SeekWrapper) Seek(offset int64, whence int) (int64, error) { + switch whence { + case os.SEEK_CUR: + return 0, nil + case os.SEEK_END: + return file.Size, nil + } + return 0, nil +} + +// Interfaces that SeekWrapper implements +var _ io.Reader = (*SeekWrapper)(nil) +var _ io.Seeker = (*SeekWrapper)(nil) diff --git a/googlecloudstorage/googlecloudstorage.go b/googlecloudstorage/googlecloudstorage.go index 42a19b877..a49aa3cdd 100644 --- a/googlecloudstorage/googlecloudstorage.go +++ b/googlecloudstorage/googlecloudstorage.go @@ -359,8 +359,9 @@ func (f *FsStorage) ListDir() fs.DirChan { // The new object may have been created if an error is returned func (f *FsStorage) Put(in io.Reader, remote string, modTime time.Time, size int64) (fs.Object, error) { // Temporary FsObject under construction - fs := &FsObjectStorage{storage: f, remote: remote} - return fs, fs.Update(in, modTime, size) + o := &FsObjectStorage{storage: f, remote: remote} + in = &fs.SeekWrapper{In: in, Size: size} + return o, o.Update(in, modTime, size) } // Mkdir creates the bucket if it doesn't exist @@ -562,6 +563,7 @@ func (o *FsObjectStorage) Update(in io.Reader, modTime time.Time, size int64) er Updated: modTime.Format(timeFormatOut), // Doesn't get set Metadata: metadataFromModTime(modTime), } + in = &fs.SeekWrapper{In: in, Size: size} newObject, err := o.storage.svc.Objects.Insert(o.storage.bucket, &object).Media(in).Name(object.Name).PredefinedAcl(o.storage.objectAcl).Do() if err != nil { return err