From 2496b7cd1b4a4769db88cb3251a4bcb2d9d3383c Mon Sep 17 00:00:00 2001
From: Juan Batiz-Benet <juan@benet.ai>
Date: Tue, 20 Jan 2015 04:11:01 -0800
Subject: [PATCH] AddChild() can now happen while proc is Closing()

There was possibility for a deadlock otherwise. See:
https://github.com/jbenet/goprocess/issues/1
---
 goprocess.go  |  2 +-
 impl-mutex.go | 31 ++++++++++++++++++++++---------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/goprocess.go b/goprocess.go
index afe848c..762cecb 100644
--- a/goprocess.go
+++ b/goprocess.go
@@ -18,7 +18,7 @@ import (
 // More specifically, it fits this:
 //
 //   p := WithTeardown(tf) // new process is created, it is now running.
-//   p.AddChild(q)         // can register children **before** Closing.
+//   p.AddChild(q)         // can register children **before** Closed().
 //   go p.Close()          // blocks until done running teardown func.
 //   <-p.Closing()         // would now return true.
 //   <-p.childrenDone()    // wait on all children to be done
diff --git a/impl-mutex.go b/impl-mutex.go
index ed68b9a..633d5b0 100644
--- a/impl-mutex.go
+++ b/impl-mutex.go
@@ -92,16 +92,18 @@ func (p *process) Go(f ProcessFunc) Process {
 // it's a wrapper around internalClose that waits on Closed()
 func (p *process) Close() error {
 	p.Lock()
-	defer p.Unlock()
 
-	// if already closed, get out.
+	// if already closing, or closed, get out. (but wait!)
 	select {
-	case <-p.Closed():
+	case <-p.Closing():
+		p.Unlock()
+		<-p.Closed()
 		return p.closeErr
 	default:
 	}
 
 	p.doClose()
+	p.Unlock()
 	return p.closeErr
 }
 
@@ -120,12 +122,23 @@ func (p *process) doClose() {
 
 	close(p.closing) // signal that we're shutting down (Closing)
 
-	for _, c := range p.children {
-		go c.Close() // force all children to shut down
-	}
-
-	for _, w := range p.waitfors {
-		<-w.Closed() // wait till all waitfors are fully closed (before teardown)
+	for len(p.children) > 0 || len(p.waitfors) > 0 {
+		for _, c := range p.children {
+			go c.Close() // force all children to shut down
+		}
+		p.children = nil // clear them
+
+		// we must be careful not to iterate over waitfors directly, as it may
+		// change under our feet.
+		wf := p.waitfors
+		p.waitfors = nil // clear them
+		for _, w := range wf {
+			// Here, we wait UNLOCKED, so that waitfors who are in the middle of
+			// adding a child to us can finish. we will immediately close the child.
+			p.Unlock()
+			<-w.Closed() // wait till all waitfors are fully closed (before teardown)
+			p.Lock()
+		}
 	}
 
 	p.closeErr = p.teardown() // actually run the close logic (ok safe to teardown)