diff --git a/CHANGELOG.md b/CHANGELOG.md index e2f2a10ee..e0e1bcf95 100644 --- a/CHANGELOG.md +++ b/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] diff --git a/mfs/dir.go b/mfs/dir.go index 38302ac39..581f446a6 100644 --- a/mfs/dir.go +++ b/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 } diff --git a/mfs/file.go b/mfs/file.go index aff025db6..700244321 100644 --- a/mfs/file.go +++ b/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}) } diff --git a/mfs/root.go b/mfs/root.go index e584b6e06..e332f2da3 100644 --- a/mfs/root.go +++ b/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