Merge pull request #42 from smola/tmp-fixes

 util.TempFile: use ioutil-like implementation, fixes #41

Author: mcuadros

helper/temporal/temporal_test.go

@@ -15,11 +15,12 @@ func Test(t *testing.T) { TestingT(t) }
 var _ = Suite(&TemporalSuite{})
 
 type TemporalSuite struct {
-	test.TempFileSuite
+	test.FilesystemSuite
 }
 
 func (s *TemporalSuite) SetUpTest(c *C) {
-	s.FS = New(memfs.New(), "foo")
+	fs := New(memfs.New(), "foo")
+	s.FilesystemSuite = test.NewFilesystemSuite(fs)
 }
 
 func (s *TemporalSuite) TestTempFileDefaultPath(c *C) {

test/tempfile.go

@@ -5,6 +5,7 @@ import (
 
 	. "gopkg.in/check.v1"
 	"gopkg.in/src-d/go-billy.v3"
+	"gopkg.in/src-d/go-billy.v3/util"
 )
 
 // TempFileSuite is a convenient test suite to validate any implementation of
@@ -45,7 +46,49 @@ func (s *TempFileSuite) TestRemoveTempFile(c *C) {
 	c.Assert(err, IsNil)
 
 	fn := f.Name()
-	c.Assert(err, IsNil)
 	c.Assert(f.Close(), IsNil)
 	c.Assert(s.FS.Remove(fn), IsNil)
 }
+
+func (s *TempFileSuite) TestRenameTempFile(c *C) {
+	f, err := s.FS.TempFile("test-dir", "test-prefix")
+	c.Assert(err, IsNil)
+
+	fn := f.Name()
+	c.Assert(f.Close(), IsNil)
+	c.Assert(s.FS.Rename(fn, "other-path"), IsNil)
+}
+
+func (s *TempFileSuite) TestTempFileMany(c *C) {
+	for i := 0; i < 1024; i++ {
+		var fs []billy.File
+
+		for j := 0; j < 100; j++ {
+			f, err := s.FS.TempFile("test-dir", "test-prefix")
+			c.Assert(err, IsNil)
+			fs = append(fs, f)
+		}
+
+		for _, f := range fs {
+			c.Assert(f.Close(), IsNil)
+			c.Assert(s.FS.Remove(f.Name()), IsNil)
+		}
+	}
+}
+
+func (s *TempFileSuite) TestTempFileManyWithUtil(c *C) {
+	for i := 0; i < 1024; i++ {
+		var fs []billy.File
+
+		for j := 0; j < 100; j++ {
+			f, err := util.TempFile(s.FS, "test-dir", "test-prefix")
+			c.Assert(err, IsNil)
+			fs = append(fs, f)
+		}
+
+		for _, f := range fs {
+			c.Assert(f.Close(), IsNil)
+			c.Assert(s.FS.Remove(f.Name()), IsNil)
+		}
+	}
+}

util/util.go

@@ -1,11 +1,11 @@
 package util
 
 import (
-	"errors"
-	"fmt"
 	"io"
 	"os"
-	"sync/atomic"
+	"path/filepath"
+	"strconv"
+	"sync"
 	"time"
 
 	"gopkg.in/src-d/go-billy.v3"
@@ -114,38 +114,58 @@ func WriteFile(fs billy.Basic, filename string, data []byte, perm os.FileMode) e
 	return err
 }
 
-var (
-	MaxTempFiles int32 = 1024 * 4
-	tempCount    int32
-)
+// Random number state.
+// We generate random temporary file names so that there's a good
+// chance the file doesn't exist yet - keeps the number of tries in
+// TempFile to a minimum.
+var rand uint32
+var randmu sync.Mutex
+
+func reseed() uint32 {
+	return uint32(time.Now().UnixNano() + int64(os.Getpid()))
+}
+
+func nextSuffix() string {
+	randmu.Lock()
+	r := rand
+	if r == 0 {
+		r = reseed()
+	}
+	r = r*1664525 + 1013904223 // constants from Numerical Recipes
+	rand = r
+	randmu.Unlock()
+	return strconv.Itoa(int(1e9 + r%1e9))[1:]
+}
 
 // TempFile creates a new temporary file in the directory dir with a name
 // beginning with prefix, opens the file for reading and writing, and returns
 // the resulting *os.File. If dir is the empty string, TempFile uses the default
-// directory for temporary files (see os.TempDir).
-//
-// Multiple programs calling TempFile simultaneously will not choose the same
-// file. The caller can use f.Name() to find the pathname of the file.
-//
-// It is the caller's responsibility to remove the file when no longer needed.
-func TempFile(fs billy.Basic, dir, prefix string) (billy.File, error) {
-	var fullpath string
-	for {
-		if tempCount >= MaxTempFiles {
-			return nil, errors.New("max. number of tempfiles reached")
-		}
+// directory for temporary files (see os.TempDir). Multiple programs calling
+// TempFile simultaneously will not choose the same file. The caller can use
+// f.Name() to find the pathname of the file. It is the caller's responsibility
+// to remove the file when no longer needed.
+func TempFile(fs billy.Basic, dir, prefix string) (f billy.File, err error) {
+	// This implementation is based on stdlib ioutil.TempFile.
+
+	if dir == "" {
+		dir = os.TempDir()
+	}
 
-		fullpath = getTempFilename(fs, dir, prefix)
+	nconflict := 0
+	for i := 0; i < 10000; i++ {
+		name := filepath.Join(dir, prefix+nextSuffix())
+		f, err = fs.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0600)
+		if os.IsExist(err) {
+			if nconflict++; nconflict > 10 {
+				randmu.Lock()
+				rand = reseed()
+				randmu.Unlock()
+			}
+			continue
+		}
 		break
 	}
-
-	return fs.Create(fullpath)
-}
-
-func getTempFilename(fs billy.Basic, dir, prefix string) string {
-	atomic.AddInt32(&tempCount, 1)
-	filename := fmt.Sprintf("%s_%d_%d", prefix, tempCount, time.Now().UnixNano())
-	return fs.Join(dir, filename)
+	return
 }
 
 type underlying interface {