19192: Fix deadlock in lookupnode.

[arvados.git] / sdk / go / arvados / fs_lookup.go

diff --git a/sdk/go/arvados/fs_lookup.go b/sdk/go/arvados/fs_lookup.go
index 42322a14a9adda155c75f225ef43e2cdfd96615c..2bb09995e16e476b3889abbdc1c61b6fc1abbd15 100644 (file)
--- a/sdk/go/arvados/fs_lookup.go
+++ b/sdk/go/arvados/fs_lookup.go
@@ -6,7 +6,6 @@ package arvados
 
 import (
        "os"
-       "sync"
        "time"
 )
 
@@ -15,31 +14,45 @@ import (
 //
 // See (*customFileSystem)MountUsers for example usage.
 type lookupnode struct {
-       inode
+       treenode
        loadOne func(parent inode, name string) (inode, error)
        loadAll func(parent inode) ([]inode, error)
        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,
+// triggers a reload on next lookup.
+func (ln *lookupnode) Sync() error {
+       err := ln.treenode.Sync()
+       if err != nil {
+               return err
+       }
+       ln.Lock()
+       ln.staleAll = time.Time{}
+       ln.staleOne = nil
+       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 {
-                       _, err = ln.inode.Child(child.FileInfo().Name(), func(inode) (inode, error) {
+                       _, err = ln.treenode.Child(child.FileInfo().Name(), func(inode) (inode, error) {
                                return child, nil
                        })
                        if err != nil {
+                               ln.Unlock()
                                return nil, err
                        }
                }
@@ -49,25 +62,46 @@ func (ln *lookupnode) Readdir() ([]os.FileInfo, error) {
                // newer than ln.staleAll. Reclaim memory.
                ln.staleOne = nil
        }
-       return ln.inode.Readdir()
+       ln.Unlock()
+       return ln.treenode.Readdir()
 }
 
+// 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
        if ln.stale(ln.staleAll) && ln.stale(ln.staleOne[name]) {
-               _, err := ln.inode.Child(name, func(inode) (inode, error) {
+               existing, err = ln.treenode.Child(name, func(inode) (inode, error) {
                        return ln.loadOne(ln, name)
                })
-               if err != nil {
-                       return nil, err
+               if err == nil && existing != nil {
+                       if ln.staleOne == nil {
+                               ln.staleOne = map[string]time.Time{name: checkTime}
+                       } else {
+                               ln.staleOne[name] = checkTime
+                       }
                }
-               if ln.staleOne == nil {
-                       ln.staleOne = map[string]time.Time{name: checkTime}
-               } else {
-                       ln.staleOne[name] = checkTime
+       } else {
+               existing, err = ln.treenode.Child(name, nil)
+               if err != nil && !os.IsNotExist(err) {
+                       return existing, err
+               }
+       }
+       if replace != nil {
+               // Let the callback try to delete or replace the
+               // existing node; if it does, return
+               // ErrInvalidOperation.
+               if tryRepl, err := replace(existing); err != nil {
+                       // Propagate error from callback
+                       return existing, err
+               } else if tryRepl != existing {
+                       return existing, ErrInvalidOperation
                }
        }
-       return ln.inode.Child(name, replace)
+       // Return original error from ln.treenode.Child() (it might be
+       // ErrNotExist).
+       return existing, err
 }