fs: return deterministic result in Readdir

This avoids that the result from concurrent directory reads (which
would return entries in different order) pollute kernel caches. Add a
test.

Fixes #391

Change-Id: I06a252bd

fs/bridge.go

@@ -975,11 +975,12 @@ func (b *rawBridge) getStream(ctx context.Context, inode *Inode) (DirStream, sys
 		return rd.Readdir(ctx)
 	}
 
-	r := []fuse.DirEntry{}
-	for k, ch := range inode.Children() {
-		r = append(r, fuse.DirEntry{Mode: ch.Mode(),
-			Name: k,
-			Ino:  ch.StableAttr().Ino})
+	lst := inode.childrenList()
+	r := make([]fuse.DirEntry, 0, len(lst))
+	for _, e := range lst {
+		r = append(r, fuse.DirEntry{Mode: e.Inode.Mode(),
+			Name: e.Name,
+			Ino:  e.Inode.StableAttr().Ino})
 	}
 	return NewListDirStream(r), 0
 }

fs/inode.go

@@ -511,6 +511,41 @@ func (n *Inode) Children() map[string]*Inode {
 	return r
 }
 
+type childEntry struct {
+	Name  string
+	Inode *Inode
+}
+
+// childrenList returns the list of children of this directory Inode.
+// The result is guaranteed to be stable as long as the directory did
+// not change.
+func (n *Inode) childrenList() []childEntry {
+	n.mu.Lock()
+	defer n.mu.Unlock()
+	r := make([]childEntry, 0, 2*len(n.children))
+
+	// The spec doesn't guarantee this, but as long as maps remain
+	// backed by hash tables, the simplest mechanism for
+	// randomization is picking a random start index. We undo this
+	// here by picking a deterministic start index again. If the
+	// Go runtime ever implements a memory moving GC, we might
+	// have to look at the keys instead.
+	minNode := ^uintptr(0)
+	minIdx := -1
+	for k, v := range n.children {
+		if p := uintptr(unsafe.Pointer(v)); p < minNode {
+			minIdx = len(r)
+			minNode = p
+		}
+		r = append(r, childEntry{Name: k, Inode: v})
+	}
+
+	if minIdx > 0 {
+		r = append(r[minIdx:], r[:minIdx]...)
+	}
+	return r
+}
+
 // Parents returns a parent of this Inode, or nil if this Inode is
 // deleted or is the root
 func (n *Inode) Parent() (string, *Inode) {

fs/mem_test.go

@@ -7,11 +7,15 @@ package fs
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io/ioutil"
 	"math/rand"
 	"os"
+	"reflect"
+	"sync"
 	"syscall"
 	"testing"
+	"time"
 
 	"github.com/hanwen/go-fuse/v2/fuse"
 	"github.com/hanwen/go-fuse/v2/internal/testutil"
@@ -217,3 +221,69 @@ func TestDataSymlink(t *testing.T) {
 		t.Errorf("Readlink: got %q want %q", got, want)
 	}
 }
+
+func TestReaddirplusParallel(t *testing.T) {
+	root := &Inode{}
+	N := 100
+	oneSec := time.Second
+	names := map[string]int64{}
+	mntDir, _, clean := testMount(t, root, &Options{
+		FirstAutomaticIno: 1,
+		EntryTimeout:      &oneSec,
+		AttrTimeout:       &oneSec,
+		OnAdd: func(ctx context.Context) {
+			n := root.EmbeddedInode()
+
+			for i := 0; i < N; i++ {
+				ch := n.NewPersistentInode(
+					ctx,
+					&MemRegularFile{
+						Data: bytes.Repeat([]byte{'x'}, i),
+					},
+					StableAttr{})
+
+				name := fmt.Sprintf("file%04d", i)
+				names[name] = int64(i)
+				n.AddChild(name, ch, false)
+			}
+		},
+	})
+	defer clean()
+
+	read := func() (map[string]int64, error) {
+		es, err := os.ReadDir(mntDir)
+		if err != nil {
+			return nil, err
+		}
+
+		r := map[string]int64{}
+		for _, e := range es {
+			inf, err := e.Info()
+			if err != nil {
+				return nil, err
+			}
+			r[e.Name()] = inf.Size()
+		}
+		return r, nil
+	}
+
+	var wg sync.WaitGroup
+	for i := 0; i < N; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			res, err := read()
+			if err != nil {
+				t.Errorf("readdir: %v", err)
+			}
+			if got, want := len(res), len(names); got != want {
+				t.Errorf("got %d want %d", got, want)
+				return
+			}
+			if !reflect.DeepEqual(res, names) {
+				t.Errorf("maps have different content")
+			}
+		}()
+	}
+	wg.Wait()
+}