From 56982b4b3ada89a1eb638ec28f3dcfb47f581b44 Mon Sep 17 00:00:00 2001
From: Jeromy <jeromyj@gmail.com>
Date: Fri, 18 Dec 2015 21:59:21 -0800
Subject: [PATCH] do not hold locks for multiple filesystem nodes at the same
 time

License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
---
 mfs/dir.go    | 39 +++++++++++++++++++++++++++++----------
 mfs/file.go   | 44 ++++++++++++++++++++++++++++++--------------
 mfs/system.go | 17 +++++++++++++++++
 3 files changed, 76 insertions(+), 24 deletions(-)

diff --git a/mfs/dir.go b/mfs/dir.go
index 946d9e9a4..8ca79e74a 100644
--- a/mfs/dir.go
+++ b/mfs/dir.go
@@ -50,19 +50,34 @@ func NewDirectory(ctx context.Context, name string, node *dag.Node, parent child
 // closeChild updates the child by the given name to the dag node 'nd'
 // and changes its own dag node, then propogates the changes upward
 func (d *Directory) closeChild(name string, nd *dag.Node) error {
-	_, err := d.dserv.Add(nd)
+	mynd, err := d.closeChildUpdate(name, nd)
 	if err != nil {
 		return err
 	}
 
+	return d.parent.closeChild(d.name, mynd)
+}
+
+// closeChildUpdate is the portion of closeChild that needs to be locked around
+func (d *Directory) closeChildUpdate(name string, nd *dag.Node) (*dag.Node, error) {
 	d.lock.Lock()
 	defer d.lock.Unlock()
-	err = d.updateChild(name, nd)
+
+	err := d.updateChild(name, nd)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	return d.parent.closeChild(d.name, d.node)
+	return d.flushCurrentNode()
+}
+
+func (d *Directory) flushCurrentNode() (*dag.Node, error) {
+	_, err := d.dserv.Add(d.node)
+	if err != nil {
+		return nil, err
+	}
+
+	return d.node.Copy(), nil
 }
 
 func (d *Directory) updateChild(name string, nd *dag.Node) error {
@@ -263,7 +278,7 @@ func (d *Directory) Mkdir(name string) (*Directory, error) {
 		return nil, err
 	}
 
-	err = d.parent.closeChild(d.name, d.node)
+	err = d.flushUp()
 	if err != nil {
 		return nil, err
 	}
@@ -285,13 +300,18 @@ func (d *Directory) Unlink(name string) error {
 		return err
 	}
 
+	return d.flushUp()
+}
+
+func (d *Directory) flushUp() error {
+
 	return d.parent.closeChild(d.name, d.node)
 }
 
 // AddChild adds the node 'nd' under this directory giving it the name 'name'
 func (d *Directory) AddChild(name string, nd *dag.Node) error {
-	d.Lock()
-	defer d.Unlock()
+	d.lock.Lock()
+	defer d.lock.Unlock()
 
 	_, err := d.childUnsync(name)
 	if err == nil {
@@ -310,7 +330,6 @@ func (d *Directory) AddChild(name string, nd *dag.Node) error {
 
 	d.modTime = time.Now()
 
-	//return d.parent.closeChild(d.name, d.node)
 	return nil
 }
 
@@ -353,8 +372,8 @@ func (d *Directory) sync() error {
 }
 
 func (d *Directory) GetNode() (*dag.Node, error) {
-	d.Lock()
-	defer d.Unlock()
+	d.lock.Lock()
+	defer d.lock.Unlock()
 
 	err := d.sync()
 	if err != nil {
diff --git a/mfs/file.go b/mfs/file.go
index fea1112dc..8539a253f 100644
--- a/mfs/file.go
+++ b/mfs/file.go
@@ -16,8 +16,9 @@ type File struct {
 	name       string
 	hasChanges bool
 
-	mod  *mod.DagModifier
-	lock sync.Mutex
+	dserv dag.DAGService
+	mod   *mod.DagModifier
+	lock  sync.Mutex
 }
 
 // NewFile returns a NewFile object with the given parameters
@@ -28,6 +29,7 @@ func NewFile(name string, node *dag.Node, parent childCloser, dserv dag.DAGServi
 	}
 
 	return &File{
+		dserv:  dserv,
 		parent: parent,
 		name:   name,
 		mod:    dmod,
@@ -60,29 +62,43 @@ func (fi *File) CtxReadFull(ctx context.Context, b []byte) (int, error) {
 // and signals a republish to occur
 func (fi *File) Close() error {
 	fi.Lock()
-	defer fi.Unlock()
 	if fi.hasChanges {
 		err := fi.mod.Sync()
 		if err != nil {
 			return err
 		}
 
-		nd, err := fi.mod.GetNode()
-		if err != nil {
-			return err
-		}
+		fi.hasChanges = false
+
+		// explicitly stay locked for flushUp call,
+		// it will manage the lock for us
+		return fi.flushUp()
+	}
+
+	return nil
+}
 
+// flushUp syncs the file and adds it to the dagservice
+// it *must* be called with the File's lock taken
+func (fi *File) flushUp() error {
+	nd, err := fi.mod.GetNode()
+	if err != nil {
 		fi.Unlock()
-		err = fi.parent.closeChild(fi.name, nd)
-		fi.Lock()
-		if err != nil {
-			return err
-		}
+		return err
+	}
 
-		fi.hasChanges = false
+	_, err = fi.dserv.Add(nd)
+	if err != nil {
+		fi.Unlock()
+		return err
 	}
 
-	return nil
+	name := fi.name
+	parent := fi.parent
+
+	// explicit unlock *only* before closeChild call
+	fi.Unlock()
+	return parent.closeChild(name, nd)
 }
 
 // Sync flushes the changes in the file to disk
diff --git a/mfs/system.go b/mfs/system.go
index 2cfc4e201..d3e705273 100644
--- a/mfs/system.go
+++ b/mfs/system.go
@@ -109,6 +109,23 @@ func (kr *Root) GetValue() FSNode {
 	return kr.val
 }
 
+func (kr *Root) Flush() error {
+	nd, err := kr.GetValue().GetNode()
+	if err != nil {
+		return err
+	}
+
+	k, err := kr.dserv.Add(nd)
+	if err != nil {
+		return err
+	}
+
+	if kr.repub != nil {
+		kr.repub.Update(k)
+	}
+	return nil
+}
+
 // closeChild implements the childCloser interface, and signals to the publisher that
 // there are changes ready to be published
 func (kr *Root) closeChild(name string, nd *dag.Node) error {
-- 
GitLab