diff options
| author | Shulhan <ms@kilabit.info> | 2021-09-12 14:49:57 +0700 |
|---|---|---|
| committer | Shulhan <ms@kilabit.info> | 2021-09-13 00:30:28 +0700 |
| commit | f8efb407cb46d42315b3ac4088bb34e77fe6523a (patch) | |
| tree | 82c6648f2793becacf4b6397ded7ec39cb9ebf0d | |
| parent | 0fde2ddea4c6ae6b5494e79f79b975e802992c84 (diff) | |
| download | pakakeh.go-f8efb407cb46d42315b3ac4088bb34e77fe6523a.tar.xz | |
lib/memfs: fix possible data race on PathNode
During Memfs Get(), if the node returned by PathNodes.Get() is null,
the memfs instance will try to refresh the directory tree. In case
the requested path exist, the memfs will write to PathNodes through
AddChild()
At the same time, there maybe a request to access another path, which
cause both read and write occured.
| -rw-r--r-- | lib/memfs/memfs.go | 26 | ||||
| -rw-r--r-- | lib/memfs/pathnode.go | 50 |
2 files changed, 62 insertions, 14 deletions
diff --git a/lib/memfs/memfs.go b/lib/memfs/memfs.go index 0e248099..57bfb5c3 100644 --- a/lib/memfs/memfs.go +++ b/lib/memfs/memfs.go @@ -65,7 +65,7 @@ func Merge(params ...*MemFS) (merged *MemFS) { Opts: &Options{}, } - merged.PathNodes.v["/"] = merged.Root + merged.PathNodes.Set("/", merged.Root) for _, mfs := range params { for _, child := range mfs.Root.Childs { @@ -75,13 +75,14 @@ func Merge(params ...*MemFS) (merged *MemFS) { } merged.Root.AddChild(child) } - for path, node := range mfs.PathNodes.v { + paths := mfs.PathNodes.Paths() + for _, path := range paths { if path == "/" { continue } _, exist := merged.PathNodes.v[path] if !exist { - merged.PathNodes.v[path] = node + merged.PathNodes.v[path] = mfs.PathNodes.Get(path) } } } @@ -145,7 +146,7 @@ func (mfs *MemFS) AddChild(parent *Node, fi os.FileInfo) (child *Node, err error return nil, nil } - mfs.PathNodes.v[child.Path] = child + mfs.PathNodes.Set(child.Path, child) return child, nil } @@ -196,7 +197,7 @@ func (mfs *MemFS) AddFile(internalPath, externalPath string) (*Node, error) { parent.Childs = append(parent.Childs, node) } - mfs.PathNodes.v[node.Path] = node + mfs.PathNodes.Set(node.Path, node) parent = node } @@ -224,7 +225,7 @@ func (mfs *MemFS) AddFile(internalPath, externalPath string) (*Node, error) { } parent.Childs = append(parent.Childs, node) - mfs.PathNodes.v[node.Path] = node + mfs.PathNodes.Set(node.Path, node) return node, nil } @@ -253,7 +254,8 @@ func (mfs *MemFS) ContentEncode(encoding string) (err error) { return fmt.Errorf("memfs.ContentEncode: invalid encoding " + encoding) } - for _, node := range mfs.PathNodes.v { + nodes := mfs.PathNodes.Nodes() + for _, node := range nodes { if node.mode.IsDir() || len(node.V) == 0 { continue } @@ -323,7 +325,8 @@ func (mfs *MemFS) ListNames() (paths []string) { paths = append(paths, k) } - for k := range mfs.PathNodes.v { + vpaths := mfs.PathNodes.Paths() + for _, k := range vpaths { _, ok := mfs.PathNodes.f[k] if !ok { paths = append(paths, k) @@ -370,7 +373,7 @@ func (mfs *MemFS) Open(path string) (http.File, error) { func (mfs *MemFS) RemoveChild(parent *Node, child *Node) (removed *Node) { removed = parent.removeChild(child) if removed != nil { - delete(mfs.PathNodes.v, removed.Path) + mfs.PathNodes.Delete(removed.Path) } return } @@ -391,7 +394,8 @@ func (mfs *MemFS) Search(words []string, snippetLen int) (results []SearchResult tokens[x] = bytes.ToLower(tokens[x]) } - for _, node := range mfs.PathNodes.v { + nodes := mfs.PathNodes.Nodes() + for _, node := range nodes { if node.mode.IsDir() { continue } @@ -479,7 +483,7 @@ func (mfs *MemFS) createRoot(f *os.File) error { } mfs.Root.generateFuncName(mfs.Opts.Root) - mfs.PathNodes.v[mfs.Root.Path] = mfs.Root + mfs.PathNodes.Set(mfs.Root.Path, mfs.Root) return nil } diff --git a/lib/memfs/pathnode.go b/lib/memfs/pathnode.go index 8774084e..68f58bae 100644 --- a/lib/memfs/pathnode.go +++ b/lib/memfs/pathnode.go @@ -8,14 +8,16 @@ import ( "bytes" "fmt" "sort" + "sync" ) // // PathNode contains a mapping between path and Node. // type PathNode struct { - v map[string]*Node - f map[string]func() *Node + mu sync.Mutex + v map[string]*Node + f map[string]func() *Node } // @@ -29,9 +31,21 @@ func NewPathNode() *PathNode { } // +// Delete the the node by its path. +// +func (pn *PathNode) Delete(path string) { + pn.mu.Lock() + delete(pn.v, path) + pn.mu.Unlock() +} + +// // Get the node by path, or nil if path is not exist. // func (pn *PathNode) Get(path string) *Node { + pn.mu.Lock() + defer pn.mu.Unlock() + node, ok := pn.v[path] if ok { return node @@ -39,13 +53,17 @@ func (pn *PathNode) Get(path string) *Node { if pn.f != nil { f, ok := pn.f[path] if ok { - return f() + node = f() + return node } } return nil } func (pn *PathNode) MarshalJSON() ([]byte, error) { + pn.mu.Lock() + defer pn.mu.Unlock() + // Merge the path with function to node into v. for k, fn := range pn.f { pn.v[k] = fn() @@ -75,11 +93,37 @@ func (pn *PathNode) MarshalJSON() ([]byte, error) { } // +// Nodes return all the nodes. +// +func (pn *PathNode) Nodes() (nodes []*Node) { + pn.mu.Lock() + for _, node := range pn.v { + nodes = append(nodes, node) + } + pn.mu.Unlock() + return nodes +} + +// +// Paths return all the nodes keys as list of path. +// +func (pn *PathNode) Paths() (paths []string) { + pn.mu.Lock() + for key := range pn.v { + paths = append(paths, key) + } + pn.mu.Unlock() + return paths +} + +// // Set mapping of path to Node. // func (pn *PathNode) Set(path string, node *Node) { if len(path) == 0 || node == nil { return } + pn.mu.Lock() pn.v[path] = node + pn.mu.Unlock() } |
