Skip to content

Commit

Permalink
protofsm: fix race in state machine executor tests
Browse files Browse the repository at this point in the history
In this commit, we fix an existing race in the new `protofsm` state
machine executor tests.

The race would appear as such:
```
--- FAIL: TestStateMachineMsgMapper (0.00s)
    state_machine_test.go:165:
        Error Trace:/home/runner/work/lnd/lnd/protofsm/state_machine_test.go:165
                    /home/runner/work/lnd/lnd/protofsm/state_machine_test.go:451
        Error:      Object expected to be of type *protofsm.dummyStateStart, but was *protofsm.dummyStateFin
        Test:       TestStateMachineMsgMapper
FAIL
FAILgithub.com/lightningnetwork/lnd/protofsm0.116s
FAIL
```

This race condition was triggered as before we would start the state
machine _then_ register for notifications. In `Start` we emit the
starting event, then enter the main loop. If that event gets emitted
before our subscription, then we'll miss the event, as the terminal
event will be the only one received.

We fix this by registering for the events before the daemon has started.
  • Loading branch information
Roasbeef committed Nov 27, 2024
1 parent fbeab72 commit 0ff0ac8
Showing 1 changed file with 14 additions and 9 deletions.
23 changes: 14 additions & 9 deletions protofsm/state_machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,14 +251,14 @@ func TestStateMachineOnInitDaemonEvent(t *testing.T) {
// message adapter is called on start up.
adapters.On("SendMessages", *pub1, mock.Anything).Return(nil)

stateMachine.Start()
defer stateMachine.Stop()

// As we're triggering internal events, we'll also subscribe to the set
// of new states so we can assert as we go.
stateSub := stateMachine.RegisterStateEvents()
defer stateMachine.RemoveStateSub(stateSub)

stateMachine.Start()
defer stateMachine.Stop()

// Assert that we go from the starting state to the final state. The
// state machine should now also be on the final terminal state.
expectedStates := []State[dummyEvents, *dummyEnv]{
Expand Down Expand Up @@ -292,14 +292,15 @@ func TestStateMachineInternalEvents(t *testing.T) {
InitEvent: fn.None[DaemonEvent](),
}
stateMachine := NewStateMachine(cfg)
stateMachine.Start()
defer stateMachine.Stop()

// As we're triggering internal events, we'll also subscribe to the set
// of new states so we can assert as we go.
stateSub := stateMachine.RegisterStateEvents()
defer stateMachine.RemoveStateSub(stateSub)

stateMachine.Start()
defer stateMachine.Stop()

// For this transition, we'll send in the emitInternal event, which'll
// send us back to the starting event, but emit an internal event.
stateMachine.SendEvent(&emitInternal{})
Expand Down Expand Up @@ -343,14 +344,15 @@ func TestStateMachineDaemonEvents(t *testing.T) {
InitEvent: fn.None[DaemonEvent](),
}
stateMachine := NewStateMachine(cfg)
stateMachine.Start()
defer stateMachine.Stop()

// As we're triggering internal events, we'll also subscribe to the set
// of new states so we can assert as we go.
stateSub := stateMachine.RegisterStateEvents()
defer stateMachine.RemoveStateSub(stateSub)

stateMachine.Start()
defer stateMachine.Stop()

// As soon as we send in the daemon event, we expect the
// disable+broadcast events to be processed, as they are unconditional.
adapters.On(
Expand Down Expand Up @@ -428,14 +430,17 @@ func TestStateMachineMsgMapper(t *testing.T) {
MsgMapper: fn.Some[MsgMapper[dummyEvents]](dummyMapper),
}
stateMachine := NewStateMachine(cfg)
stateMachine.Start()
defer stateMachine.Stop()

// As we're triggering internal events, we'll also subscribe to the set
// of new states so we can assert as we go.
//
// We register before calling Start to ensure we don't miss any events.
stateSub := stateMachine.RegisterStateEvents()
defer stateMachine.RemoveStateSub(stateSub)

stateMachine.Start()
defer stateMachine.Stop()

// First, we'll verify that the CanHandle method works as expected.
require.True(t, stateMachine.CanHandle(wireError))
require.False(t, stateMachine.CanHandle(&lnwire.Init{}))
Expand Down

0 comments on commit 0ff0ac8

Please sign in to comment.