Skip to content

Commit

Permalink
avoid data race caused by calling forceCompState outside commandRunt…
Browse files Browse the repository at this point in the history
…ime.Run (#3537)

Instead of startWatcher setting the component state on comm.WriteConnInfo failure, now it logs the error using the component stderr logger.

(cherry picked from commit 39f324f)
  • Loading branch information
AndersonQ authored and mergify[bot] committed Oct 6, 2023
1 parent e4cd988 commit 0a63295
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 6 deletions.
2 changes: 1 addition & 1 deletion pkg/component/runtime/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ func (c *commandRuntime) startWatcher(info *process.Info, comm Communicator) {
go func() {
err := comm.WriteConnInfo(info.Stdin)
if err != nil {
c.forceCompState(client.UnitStateFailed, fmt.Sprintf("Failed: failed to provide connection information to spawned pid '%d': %s", info.PID, err))
_, _ = c.logErr.Write([]byte(fmt.Sprintf("Failed: failed to provide connection information to spawned pid '%d': %s", info.PID, err)))
// kill instantly
_ = info.Kill()
} else {
Expand Down
11 changes: 7 additions & 4 deletions pkg/component/runtime/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2474,18 +2474,21 @@ func TestManager_FakeInput_RestartsOnMissedCheckins(t *testing.T) {
return
case state := <-sub.Ch():
t.Logf("component state changed: %+v", state)
if state.State == client.UnitStateStarting || state.State == client.UnitStateHealthy {

switch state.State {
case client.UnitStateStarting:
case client.UnitStateHealthy:
// starting and healthy are allowed
} else if state.State == client.UnitStateDegraded {
case client.UnitStateDegraded:
// should go to degraded first
wasDegraded = true
} else if state.State == client.UnitStateFailed {
case client.UnitStateFailed:
if wasDegraded {
subErrCh <- nil
} else {
subErrCh <- errors.New("should have been degraded before failed")
}
} else {
default:
subErrCh <- fmt.Errorf("unknown component state: %v", state.State)
}
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/component/runtime/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,9 @@ func (s *ComponentState) cleanupStopped() bool {
return cleaned
}

// forceState force updates the state for the entire component, forcing that state on all units.
// forceState force updates the state for the entire component, forcing that
// state on all units. It returns true if either the component state or any of
// the units state changed, false otherwise.
func (s *ComponentState) forceState(state client.UnitState, msg string) bool {
changed := false
if s.State != state || s.Message != msg {
Expand Down

0 comments on commit 0a63295

Please sign in to comment.