7121: Test mutex usage with a mock instead of time.Sleep.

diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index 3dfdce20e321bfe61ffb9b9f119e10f189ee2c48..53cf7be6ca09c62828787f1c9229cce13193b480 100644 (file)
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -14,6 +14,7 @@ import (
        "os"
        "os/signal"
        "strings"
+       "sync"
        "syscall"
        "time"
 )
@@ -132,10 +133,14 @@ func (vs *volumeSet) Set(value string) error {
        if _, err := os.Stat(value); err != nil {
                return err
        }
+       var locker sync.Locker
+       if flagSerializeIO {
+               locker = &sync.Mutex{}
+       }
        *vs = append(*vs, &UnixVolume{
-               root:      value,
-               serialize: flagSerializeIO,
-               readonly:  flagReadonly,
+               root:     value,
+               locker:   locker,
+               readonly: flagReadonly,
        })
        return nil
 }

diff --git a/services/keepstore/mutex_test.go b/services/keepstore/mutex_test.go
new file mode 100644 (file)
index 0000000..e75d910
--- /dev/null
+++ b/services/keepstore/mutex_test.go
@@ -0,0 +1,23 @@
+package main
+
+type MockMutex struct {
+       AllowLock   chan struct{}
+       AllowUnlock chan struct{}
+}
+
+func NewMockMutex() *MockMutex {
+       return &MockMutex{
+               AllowLock: make(chan struct{}),
+               AllowUnlock: make(chan struct{}),
+       }
+}
+
+// Lock waits for someone to send to AllowLock.
+func (m *MockMutex) Lock() {
+       <- m.AllowLock
+}
+
+// Unlock waits for someone to send to AllowUnlock.
+func (m *MockMutex) Unlock() {
+       <- m.AllowUnlock
+}

diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index 368ddc55f07a9c8f4b78c09ba0cd3917bbdf091a..f91861a507018b64199efd0bdc5387bbb7e0e242 100644 (file)
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -18,10 +18,12 @@ import (
 
 // A UnixVolume stores and retrieves blocks in a local directory.
 type UnixVolume struct {
-       root      string // path to the volume's root directory
-       serialize bool
-       readonly  bool
-       mutex     sync.Mutex
+       // path to the volume's root directory
+       root string
+       // something to lock during IO, typically a sync.Mutex (or nil
+       // to skip locking)
+       locker   sync.Locker
+       readonly bool
 }
 
 func (v *UnixVolume) Touch(loc string) error {
@@ -34,9 +36,9 @@ func (v *UnixVolume) Touch(loc string) error {
                return err
        }
        defer f.Close()
-       if v.serialize {
-               v.mutex.Lock()
-               defer v.mutex.Unlock()
+       if v.locker != nil {
+               v.locker.Lock()
+               defer v.locker.Unlock()
        }
        if e := lockfile(f); e != nil {
                return e
@@ -56,17 +58,17 @@ func (v *UnixVolume) Mtime(loc string) (time.Time, error) {
        }
 }
 
-// Open the given file, apply the serialize lock if enabled, and call
-// the given function if and when the file is ready to read.
+// Open the given file, lock the "serialize" locker if enabled, and
+// call the given function if and when the file is ready to read.
 func (v *UnixVolume) getFunc(path string, fn func(io.Reader) error) error {
        f, err := os.Open(path)
        if err != nil {
                return err
        }
        defer f.Close()
-       if v.serialize {
-               v.mutex.Lock()
-               defer v.mutex.Unlock()
+       if v.locker != nil {
+               v.locker.Lock()
+               defer v.locker.Unlock()
        }
        return fn(f)
 }
@@ -169,9 +171,9 @@ func (v *UnixVolume) Put(loc string, block []byte) error {
        }
        bpath := v.blockPath(loc)
 
-       if v.serialize {
-               v.mutex.Lock()
-               defer v.mutex.Unlock()
+       if v.locker != nil {
+               v.locker.Lock()
+               defer v.locker.Unlock()
        }
        if _, err := tmpfile.Write(block); err != nil {
                log.Printf("%s: writing to %s: %s\n", v, bpath, err)
@@ -298,9 +300,9 @@ func (v *UnixVolume) Delete(loc string) error {
        if v.readonly {
                return MethodDisabledError
        }
-       if v.serialize {
-               v.mutex.Lock()
-               defer v.mutex.Unlock()
+       if v.locker != nil {
+               v.locker.Lock()
+               defer v.locker.Unlock()
        }
        p := v.blockPath(loc)
        f, err := os.OpenFile(p, os.O_RDWR|os.O_APPEND, 0644)

diff --git a/services/keepstore/volume_unix_test.go b/services/keepstore/volume_unix_test.go
index 6ccc865b113d703581dc631ed7810dbeaf973bd9..08ca31cc5b639d6da8a32db79386e9c17000e976 100644 (file)
--- a/services/keepstore/volume_unix_test.go
+++ b/services/keepstore/volume_unix_test.go
@@ -10,6 +10,7 @@ import (
        "regexp"
        "sort"
        "strings"
+       "sync"
        "syscall"
        "testing"
        "time"
@@ -20,10 +21,14 @@ func TempUnixVolume(t *testing.T, serialize bool, readonly bool) *UnixVolume {
        if err != nil {
                t.Fatal(err)
        }
+       var locker sync.Locker
+       if serialize {
+               locker = &sync.Mutex{}
+       }
        return &UnixVolume{
-               root:      d,
-               serialize: serialize,
-               readonly:  readonly,
+               root:     d,
+               locker:   locker,
+               readonly: readonly,
        }
 }
 
@@ -420,23 +425,38 @@ func TestUnixVolumeGetFuncFileError(t *testing.T) {
 }
 
 func TestUnixVolumeGetFuncWorkerWaitsOnMutex(t *testing.T) {
-       v := TempUnixVolume(t, true, false)
+       v := TempUnixVolume(t, false, false)
        defer _teardown(v)
 
-       v.mutex.Lock()
-       locked := true
-       go func() {
-               // TODO(TC): Don't rely on Sleep. Mock the mutex instead?
-               time.Sleep(10 * time.Millisecond)
-               locked = false
-               v.mutex.Unlock()
-       }()
-       v.getFunc(v.blockPath(TEST_HASH), func(rdr io.Reader) error {
-               if locked {
-                       t.Errorf("Worker func called before serialize lock was obtained")
-               }
+       v.Put(TEST_HASH, TEST_BLOCK)
+
+       mtx := NewMockMutex()
+       v.locker = mtx
+
+       funcCalled := make(chan struct{})
+       go v.getFunc(v.blockPath(TEST_HASH), func(rdr io.Reader) error {
+               funcCalled <- struct{}{}
                return nil
        })
+       select {
+       case mtx.AllowLock <- struct{}{}:
+       case <-funcCalled:
+               t.Fatal("Function was called before mutex was acquired")
+       case <-time.After(5 * time.Second):
+               t.Fatal("Timed out before mutex was acquired")
+       }
+       select {
+       case <-funcCalled:
+       case mtx.AllowUnlock <- struct{}{}:
+               t.Fatal("Mutex was released before function was called")
+       case <-time.After(5 * time.Second):
+               t.Fatal("Timed out waiting for funcCalled")
+       }
+       select {
+       case mtx.AllowUnlock <- struct{}{}:
+       case <-time.After(5 * time.Second):
+               t.Fatal("Timed out waiting for getFunc() to release mutex")
+       }
 }
 
 func TestUnixVolumeCompare(t *testing.T) {