Fix a race in temp file creation. (#565)

The added test fails before the fix and passes after.

Fixes #500.

internal/driver/tempfile.go

@@ -24,9 +24,11 @@ import (
 // newTempFile returns a new output file in dir with the provided prefix and suffix.
 func newTempFile(dir, prefix, suffix string) (*os.File, error) {
 	for index := 1; index < 10000; index++ {
-		path := filepath.Join(dir, fmt.Sprintf("%s%03d%s", prefix, index, suffix))
-		if _, err := os.Stat(path); err != nil {
-			return os.Create(path)
+		switch f, err := os.OpenFile(filepath.Join(dir, fmt.Sprintf("%s%03d%s", prefix, index, suffix)), os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666); {
+		case err == nil:
+			return f, nil
+		case !os.IsExist(err):
+			return nil, err
 		}
 	}
 	// Give up
@@ -44,11 +46,15 @@ func deferDeleteTempFile(path string) {
 }
 
 // cleanupTempFiles removes any temporary files selected for deferred cleaning.
-func cleanupTempFiles() {
+func cleanupTempFiles() error {
 	tempFilesMu.Lock()
+	defer tempFilesMu.Unlock()
+	var lastErr error
 	for _, f := range tempFiles {
-		os.Remove(f)
+		if err := os.Remove(f); err != nil {
+			lastErr = err
+		}
 	}
 	tempFiles = nil
-	tempFilesMu.Unlock()
+	return lastErr
 }

internal/driver/tempfile_test.go

@@ -0,0 +1,55 @@
+package driver
+
+import (
+	"os"
+	"sync"
+	"testing"
+)
+
+func TestNewTempFile(t *testing.T) {
+	const n = 100
+	// Line up ready to execute goroutines with a read-write lock.
+	var mu sync.RWMutex
+	mu.Lock()
+	var wg sync.WaitGroup
+	errc := make(chan error, n)
+	for i := 0; i < n; i++ {
+		wg.Add(1)
+		go func() {
+			mu.RLock()
+			defer mu.RUnlock()
+			defer wg.Done()
+			f, err := newTempFile(os.TempDir(), "profile", ".tmp")
+			errc <- err
+			deferDeleteTempFile(f.Name())
+			f.Close()
+		}()
+	}
+	// Start the file creation race.
+	mu.Unlock()
+	// Wait for the goroutines to finish.
+	wg.Wait()
+
+	for i := 0; i < n; i++ {
+		if err := <-errc; err != nil {
+			t.Fatalf("newTempFile(): got %v, want no error", err)
+		}
+	}
+	if len(tempFiles) != n {
+		t.Errorf("len(tempFiles): got %d, want %d", len(tempFiles), n)
+	}
+	names := map[string]bool{}
+	for _, name := range tempFiles {
+		if names[name] {
+			t.Errorf("got temp file %s created multiple times", name)
+			break
+		}
+		names[name] = true
+	}
+	if err := cleanupTempFiles(); err != nil {
+		t.Errorf("cleanupTempFiles(): got error %v, want no error", err)
+	}
+	if len(tempFiles) != 0 {
+		t.Errorf("len(tempFiles) after the cleanup: got %d, want 0", len(tempFiles))
+	}
+}