diff --git a/engine/graph/engine.go b/engine/graph/engine.go index b9b4dcd6..ae884801 100644 --- a/engine/graph/engine.go +++ b/engine/graph/engine.go @@ -155,6 +155,10 @@ func (obj *Engine) Apply(fn func(*pgraph.Graph) error) error { // it errors, then the running graph wasn't changed. It is recommended that you // pause the engine before running this, and resume it after you're done. func (obj *Engine) Commit() error { + // It would be safer to lock this, but it would be slower and mask bugs. + //obj.mutex.Lock() + //defer obj.mutex.Unlock() + // TODO: Does this hurt performance or graph changes ? start := []func() error{} // functions to run after graphsync to start... @@ -349,8 +353,13 @@ func (obj *Engine) Commit() error { // Resume runs the currently active graph. It also un-pauses the graph if it was // paused. Very little that is interesting should happen here. It all happens in // the Commit method. After Commit, new things are already started, but we still -// need to Resume any pre-existing resources. +// need to Resume any pre-existing resources. Do not call this concurrently with +// the Pause method. func (obj *Engine) Resume() error { + // It would be safer to lock this, but it would be slower and mask bugs. + //obj.mutex.Lock() + //defer obj.mutex.Unlock() + if !obj.paused { return fmt.Errorf("already resumed") } @@ -385,6 +394,10 @@ func (obj *Engine) SetFastPause() { // Pause the active, running graph. func (obj *Engine) Pause(fastPause bool) error { + // It would be safer to lock this, but it would be slower and mask bugs. + //obj.mutex.Lock() + //defer obj.mutex.Unlock() + if obj.paused { return fmt.Errorf("already paused") } diff --git a/engine/graph/state.go b/engine/graph/state.go index b3eef5a6..7035f86b 100644 --- a/engine/graph/state.go +++ b/engine/graph/state.go @@ -322,11 +322,11 @@ func (obj *State) Poke() { } } -// Pause pauses this resource. It should not be called on any already paused +// Pause pauses this resource. It must not be called on any already paused // resource. It will block until the resource pauses with an acknowledgment, or // until an exit for that resource is seen. If the latter happens it will error. -// It is NOT thread-safe with the Resume() method so only call either one at a -// time. +// It must not be called concurrently with either the Resume() method or itself, +// so only call these one at a time and alternate between the two. func (obj *State) Pause() error { if obj.paused { return fmt.Errorf("already paused") @@ -349,9 +349,10 @@ func (obj *State) Pause() error { return nil } -// Resume unpauses this resource. It can be safely called on a brand-new -// resource that has just started running without incident. It is NOT -// thread-safe with the Pause() method, so only call either one at a time. +// Resume unpauses this resource. It can be safely called once on a brand-new +// resource that has just started running, without incident. It must not be +// called concurrently with either the Pause() method or itself, so only call +// these one at a time and alternate between the two. func (obj *State) Resume() { // TODO: do we need a mutex around Resume? if !obj.paused { // no need to unpause brand-new resources