Skip to content

Commit

Permalink
AddChild() can now happen while proc is Closing()
Browse files Browse the repository at this point in the history
There was possibility for a deadlock otherwise. See:
#1
  • Loading branch information
jbenet committed Jan 20, 2015
1 parent 7f96033 commit 2496b7c
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 10 deletions.
2 changes: 1 addition & 1 deletion goprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 22 additions & 9 deletions impl-mutex.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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)
Expand Down

0 comments on commit 2496b7c

Please sign in to comment.