19192: Fix deadlock in lookupnode.
[arvados.git] / sdk / go / arvados / fs_lookup.go
index 56b5953234784424e51676a90b4c148661cb8c4d..2bb09995e16e476b3889abbdc1c61b6fc1abbd15 100644 (file)
@@ -6,7 +6,6 @@ package arvados
 
 import (
        "os"
-       "sync"
        "time"
 )
 
@@ -21,9 +20,8 @@ type lookupnode struct {
        stale   func(time.Time) bool
 
        // internal fields
-       staleLock sync.Mutex
-       staleAll  time.Time
-       staleOne  map[string]time.Time
+       staleAll time.Time
+       staleOne map[string]time.Time
 }
 
 // Sync flushes pending writes for loaded children and, if successful,
@@ -33,20 +31,20 @@ func (ln *lookupnode) Sync() error {
        if err != nil {
                return err
        }
-       ln.staleLock.Lock()
+       ln.Lock()
        ln.staleAll = time.Time{}
        ln.staleOne = nil
-       ln.staleLock.Unlock()
+       ln.Unlock()
        return nil
 }
 
 func (ln *lookupnode) Readdir() ([]os.FileInfo, error) {
-       ln.staleLock.Lock()
-       defer ln.staleLock.Unlock()
+       ln.Lock()
        checkTime := time.Now()
        if ln.stale(ln.staleAll) {
                all, err := ln.loadAll(ln)
                if err != nil {
+                       ln.Unlock()
                        return nil, err
                }
                for _, child := range all {
@@ -54,6 +52,7 @@ func (ln *lookupnode) Readdir() ([]os.FileInfo, error) {
                                return child, nil
                        })
                        if err != nil {
+                               ln.Unlock()
                                return nil, err
                        }
                }
@@ -63,15 +62,14 @@ func (ln *lookupnode) Readdir() ([]os.FileInfo, error) {
                // newer than ln.staleAll. Reclaim memory.
                ln.staleOne = nil
        }
+       ln.Unlock()
        return ln.treenode.Readdir()
 }
 
-// Child rejects (with ErrInvalidArgument) calls to add/replace
+// Child rejects (with ErrInvalidOperation) calls to add/replace
 // children, instead calling loadOne when a non-existing child is
 // looked up.
 func (ln *lookupnode) Child(name string, replace func(inode) (inode, error)) (inode, error) {
-       ln.staleLock.Lock()
-       defer ln.staleLock.Unlock()
        checkTime := time.Now()
        var existing inode
        var err error
@@ -95,12 +93,12 @@ func (ln *lookupnode) Child(name string, replace func(inode) (inode, error)) (in
        if replace != nil {
                // Let the callback try to delete or replace the
                // existing node; if it does, return
-               // ErrInvalidArgument.
+               // ErrInvalidOperation.
                if tryRepl, err := replace(existing); err != nil {
                        // Propagate error from callback
                        return existing, err
                } else if tryRepl != existing {
-                       return existing, ErrInvalidArgument
+                       return existing, ErrInvalidOperation
                }
        }
        // Return original error from ln.treenode.Child() (it might be