engine: graph: Improve documentation on concurrent use

Certain other methods should not be called concurrently, but this only
documents the most important cases.
This commit is contained in:
James Shubin
2023-08-30 20:38:48 -04:00
parent b62f501745
commit 7288e5d4a4
2 changed files with 21 additions and 7 deletions

View File

@@ -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")
}

View File

@@ -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