From 52e4a6cfac46163658bd4f123c49b6ee7dc75f78 Mon Sep 17 00:00:00 2001
From: Jamie Wilkinson <jaq@spacepants.org>
Date: Tue, 20 Sep 2016 07:01:14 +1000
Subject: [PATCH] Fixes a pass-by-value error in FileData.Name()

Fixes a pass-by-value error in FileData.Name() which causes the mutex to be copied, and use that method to retrieve the name of the file from a mem.File.  This really fixes the data race that motivated PR #95. (#96)

I can't explain why moving the lock improves the situation, nor why calling through the accessor Name() instead of locking and reading f.fileData.name is not the same, but go vet indicates that the mutex in fileData was being copied, not preserved.

The reproducing test case upstream is:
check out github.com/google/mtail
make install_deps
go test -race -timeout 1m -v -run TestProcessEvents --count=10000 ./vm

Prior to this change, the test reports a data race 3 times out of 10000, after, 0 times consistently.
---
 mem/file.go    |  8 ++++----
 memmap_test.go | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/mem/file.go b/mem/file.go
index 9096ff0..3c1e09a 100644
--- a/mem/file.go
+++ b/mem/file.go
@@ -59,7 +59,9 @@ type FileData struct {
 	modtime time.Time
 }
 
-func (d FileData) Name() string {
+func (d *FileData) Name() string {
+	d.Lock()
+	defer d.Unlock()
 	return d.name
 }
 
@@ -107,9 +109,7 @@ func (f *File) Close() error {
 }
 
 func (f *File) Name() string {
-	f.fileData.Lock()
-	defer f.fileData.Unlock()
-	return f.fileData.name
+	return f.fileData.Name()
 }
 
 func (f *File) Stat() (os.FileInfo, error) {
diff --git a/memmap_test.go b/memmap_test.go
index 44d525b..9efd1f2 100644
--- a/memmap_test.go
+++ b/memmap_test.go
@@ -259,3 +259,26 @@ func TestWriteCloseTime(t *testing.T) {
 		}
 	}
 }
+
+// This test should be run with the race detector on:
+// go test -race -v -timeout 10s -run TestRacingDeleteAndClose
+func TestRacingDeleteAndClose(t *testing.T) {
+	fs := NewMemMapFs()
+	pathname := "testfile"
+	f, err := fs.Create(pathname)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	in := make(chan bool)
+
+	go func() {
+		<-in
+		f.Close()
+	}()
+	go func() {
+		<-in
+		fs.Remove(pathname)
+	}()
+	close(in)
+}