qmuntal / stateless

Go library for creating finite state machines
BSD 2-Clause "Simplified" License
942 stars 49 forks source link

accessing current state inside stateMutator #3

Closed stevenferrer closed 4 years ago

stevenferrer commented 4 years ago

Halo, thank you very much for creating this amazing project!

So, I've been experimenting on this for this past few days and I wonder if there's a possibility to access the current state inside the stateMutator?

I've tried doing it but I had an issue, coz when stateMutator gets executed (i.e. setting the next state), it acquires the lock from stateMutex, then when I try to get the current state (i.e. StateMachine.State) inside stateMutator, it calls the stateAccessor but the lock is already acquired by stateMutator, hence, deadlock.

This is specially useful coz I'm storing the state from an external storage (e.g. in-memory or db).

qmuntal commented 4 years ago

Hi @stevenferrer, it is always a pleasure to contribute :)

Can you give my more context about your use case? I'll like to know it to improve the package in future versions.

With the current implementation I don't see a way to call StateMachine.State from stateMutator. What you could do is store the reference to the stateAccessor function and call it instead, this way you bypass the mutex:

stateAccessor := func(_ context.Context) (stateless.State, error) {
  return myState.Value, nil
}
stateMutator :=  func(ctx context.Context, state stateless.State) error {
  oldState, _ := stateAccessor(ctx) 
  fmt.Println(oldState)
  myState.Value  = state
  return nil
}
stateMachine := stateless.NewStateMachineWithExternalStorage(stateAccessor, stateMutator, stateless.FiringQueued)

Does this meet your requirements?

stevenferrer commented 4 years ago

Hi @qmuntal, I've modified the phone states example here

I actually made fork and commented out the mutex lock on stateAccessor, so I can get around with the deadlock issue.

qmuntal commented 4 years ago

@stevenferrer your phone states example is really cool!

About this issue, I'll treat it as a bug. Your use case seems to be valid and pretty common but it is causing a deadlock. I'll move the mutex to the built-in in-memory state repository and remove it from the state getter and setter so the clients can decide how to guard the state.

qmuntal commented 4 years ago

Should be fixed in v1.1.0

stevenferrer commented 4 years ago

Yep, all good now!

Thanks a lot for this @qmuntal!

Closing this issue now.