Merge pull request #751 from ipfs/fix/mfs-cache-3

mfs: clean cache on sync

CHANGELOG.md

@@ -33,6 +33,8 @@ The following emojis are used to highlight certain changes:
 
 ### Fixed
 
+* `mfs`: directory cache is now cleared on Flush(), liberating the memory used by the otherwise ever-growing cache. References to directories and sub-directories should be renewed after flushing.
+
 ### Security
 
 ## [v0.25.0]

mfs/dir.go

@@ -99,10 +99,11 @@ func (d *Directory) localUpdate(c child) (*dag.ProtoNode, error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 
-	err := d.updateChild(c)
+	err := d.unixfsDir.AddChild(d.ctx, c.Name, c.Node)
 	if err != nil {
 		return nil, err
 	}
+
 	// TODO: Clearly define how are we propagating changes to lower layers
 	// like UnixFS.
 
@@ -125,31 +126,10 @@ func (d *Directory) localUpdate(c child) (*dag.ProtoNode, error) {
 	// TODO: Why do we need a copy?
 }
 
-// Update child entry in the underlying UnixFS directory.
-func (d *Directory) updateChild(c child) error {
-	err := d.unixfsDir.AddChild(d.ctx, c.Name, c.Node)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func (d *Directory) Type() NodeType {
 	return TDir
 }
 
-// childNode returns a FSNode under this directory by the given name if it exists.
-// it does *not* check the cached dirs and files
-func (d *Directory) childNode(name string) (FSNode, error) {
-	nd, err := d.childFromDag(name)
-	if err != nil {
-		return nil, err
-	}
-
-	return d.cacheNode(name, nd)
-}
-
 // cacheNode caches a node into d.childDirs or d.files and returns the FSNode.
 func (d *Directory) cacheNode(name string, nd ipld.Node) (FSNode, error) {
 	switch nd := nd.(type) {
@@ -219,7 +199,12 @@ func (d *Directory) childUnsync(name string) (FSNode, error) {
 		return entry, nil
 	}
 
-	return d.childNode(name)
+	nd, err := d.childFromDag(name)
+	if err != nil {
+		return nil, err
+	}
+
+	return d.cacheNode(name, nd)
 }
 
 type NodeListing struct {
@@ -338,7 +323,7 @@ func (d *Directory) Unlink(name string) error {
 }
 
 func (d *Directory) Flush() error {
-	nd, err := d.GetNode()
+	nd, err := d.getNode(true)
 	if err != nil {
 		return err
 	}
@@ -361,29 +346,24 @@ func (d *Directory) AddChild(name string, nd ipld.Node) error {
 		return err
 	}
 
-	err = d.unixfsDir.AddChild(d.ctx, name, nd)
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return d.unixfsDir.AddChild(d.ctx, name, nd)
 }
 
-func (d *Directory) sync() error {
+func (d *Directory) cacheSync(clean bool) error {
 	for name, entry := range d.entriesCache {
 		nd, err := entry.GetNode()
 		if err != nil {
 			return err
 		}
 
-		err = d.updateChild(child{name, nd})
+		err = d.unixfsDir.AddChild(d.ctx, name, nd)
 		if err != nil {
 			return err
 		}
 	}
-
-	// TODO: Should we clean the cache here?
-
+	if clean {
+		d.entriesCache = make(map[string]FSNode)
+	}
 	return nil
 }
 
@@ -405,10 +385,14 @@ func (d *Directory) Path() string {
 }
 
 func (d *Directory) GetNode() (ipld.Node, error) {
+	return d.getNode(false)
+}
+
+func (d *Directory) getNode(cacheClean bool) (ipld.Node, error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
 
-	err := d.sync()
+	err := d.cacheSync(cacheClean)
 	if err != nil {
 		return nil, err
 	}

mfs/file.go

@@ -270,10 +270,9 @@ func (fi *File) setNodeData(data []byte) error {
 	}
 
 	fi.nodeLock.Lock()
-	defer fi.nodeLock.Unlock()
 	fi.node = nd
 	parent := fi.inode.parent
 	name := fi.inode.name
-
+	fi.nodeLock.Unlock()
 	return parent.updateChildEntry(child{name, fi.node})
 }

mfs/root.go

@@ -170,16 +170,7 @@ func (kr *Root) Flush() error {
 func (kr *Root) FlushMemFree(ctx context.Context) error {
 	dir := kr.GetDirectory()
 
-	if err := dir.Flush(); err != nil {
-		return err
-	}
-
-	dir.lock.Lock()
-	defer dir.lock.Unlock()
-
-	clear(dir.entriesCache)
-
-	return nil
+	return dir.Flush()
 }
 
 // updateChildEntry implements the `parent` interface, and signals