Hey@ShvetsovDV , thanks for looking at it.

I think there are actually 3 different bugs that are contributing to this issue and needs to be addressed. I'm lately looking into this issue on my project and would like to help to push those fixes through. Maybe we can do it one by one?

Resuming

1. DefaultStateMachineService reset on all regions instead of top one

You did fix it alreadyhere. Going through all regions is wrong, because the reset is anyway recursive.

2. Regions are being overwritten upon reset.

Fixedhere. So all regions we iterate over are going to be always overwritten by last context.

Persisting

There is also an issue with persisting the context of regions. The context of region overwrites the main context.

Now, I'm not sure if the solution proposed is the correct one. Let me explain what I have observed. I have implemented my ownStateMachineRuntimePersister, which also implementsStateMachineInterceptor interface. There are 2 methods of our interest there:

void preStateChange(State<S, E> state, Message<E> message, Transition<S, E> transition,StateMachine<S, E> stateMachine, StateMachine<S, E> rootStateMachine);void postStateChange(State<S, E> state, Message<E> message, Transition<S, E> transition,StateMachine<S, E> stateMachine, StateMachine<S, E> rootStateMachine);

The OOTBAbstractPersistingStateMachineInterceptor implementation persists mostly inpreStateChange method. What I observed is that for thepostStateChange method,stateMachine androotStateMachine for regions are always correct and different objects.

However, forpreStateChange thestateMachine androotStateMachine are always (or almost always, I don't recall) the same object. Meaning there is a bug imho somewhere upstream.

Now the "Persisting" issues can be quickly fixed by using custom persister and persisting onpostStateChange only. HoweverResuming issues need to be fixed in the library/project itself.

So, I have two questions... do you think we can split the fixes for persisting & resuming? How can I help to push these things through? I can gladly take over.

@chutch, thanks for the answer!

Indeed, when solving the problem, several errors were fixed in different parts of the code. I think we need to commit the edits with one commit, otherwise there will be no positive effect.

I would also like to receive a comment on my commits from@jvalkeal .

However, for preStateChange the stateMachine and rootStateMachine are always (or almost always, I don't recall) the same object. Meaning there is a bug imho somewhere upstream.

Here fixed a bug in determining the machine ID for the context before saving it. Perhaps your effects are due to the fact thatbuildStateMachineContext is called not only inpreStateChange andpostStateChange.

I would be glad to receive examples of cases where my solution does not work.

OK, I'll try to rephrase to explain my concern. Let's assume that we have following simple State Machine:

            ┌─────►X1─────►X2────┐            │                    │start ───►A─┤                   ┌┘►B────►Z            │                   │            └─────►Y1─────►Y2───┘

Where:

• start, A, B, Z are states of the root State Machine, with idexample
• X1, X2 are states of a sub-machine, a region calledx therefore idexample#x
• Y1, Y2 is state of another sub-machine, a region calledy

Now, let's assume state change fromX1 ->X2. From the debugger I see that whenpostStateChange &preStateChange are called for the same state change, they are called with different parameters

preStateChange

public void preStateChange(State<S, E> state, Message<E> message, Transition<S, E> transition, StateMachine<S, E> stateMachine, StateMachine<S, E> rootStateMachine) {// with "values"state=X2stateMachine=examplerootStateMachine=example// the very same SM object, root onestateMachine == rootStateMachine

postStateChange

public void postStateChange(State<S, E> state, Message<E> message, Transition<S, E> transition, StateMachine<S, E> stateMachine, StateMachine<S, E> rootStateMachine) {// with "values"state=X2stateMachine=example#x  <== correct SM, because the state change happened within a regionrootStateMachine=example// different SM objectsstateMachine != rootStateMachine

That makes me think, that maybe trying to fix this issue by "reverse-engineering" withfindSmIdByRegion is not a proper way to fix it.

For some reason for the very same state changepostStateChange method receives proper SM objects. Then passing those 2 objects tobuildStateMachineContext() methods works perfectly fine without any changes in the code. So persisting onpostStateChange seems to be working fine.

WhereaspreStateChange receive 2x root SM, the very same object, the very same reference.

So this makes me think that maybe the fix should be performed somewhere deeper? Please mind this is just a hunch and maybe I am wrong. I think I would a lot more time to understand the inner workings of SM to see where the bug is. I am sure your code is working and doing the job. I am just wondering if it is not going to seal a deeper issue.

Let me know what you think about it. Hope this makes sense.

You are right that the error goes somewhere deeper. But the source code is difficult to write and needs a serious rewrite. Such elaboration takes time. To carry out serious rewrite without coordination and explanation of certain decisions by the owner of the source code, is fraught with new errors and loss of time.

In my implementation, I am locally parsing the structure of the statemachine in the findSmIdByRegion method. This fix will not introduce new bugs, although it looks pretty crutch.

In a serious revision, to simplify the code, I would like methods for analyzing the structure of the state machine similar to findSmIdByRegion (may be findNextState, getAvailableStates, findEventsFromSourceState, ...) into a separate static class.

Hey@ShvetsovDV , thanks for looking at it.

I think there are actually 3 different bugs that are contributing to this issue and needs to be addressed. I'm lately looking into this issue on my project and would like to help to push those fixes through. Maybe we can do it one by one?

Resuming

1. DefaultStateMachineService reset on all regions instead of top one

You did fix it alreadyhere. Going through all regions is wrong, because the reset is anyway recursive.

2. Regions are being overwritten upon reset.

Fixedhere. So all regions we iterate over are going to be always overwritten by last context.

Persisting

There is also an issue with persisting the context of regions. The context of region overwrites the main context.

Now, I'm not sure if the solution proposed is the correct one. Let me explain what I have observed. I have implemented my ownStateMachineRuntimePersister, which also implementsStateMachineInterceptor interface. There are 2 methods of our interest there:

void preStateChange(State<S, E> state, Message<E> message, Transition<S, E> transition,StateMachine<S, E> stateMachine, StateMachine<S, E> rootStateMachine);void postStateChange(State<S, E> state, Message<E> message, Transition<S, E> transition,StateMachine<S, E> stateMachine, StateMachine<S, E> rootStateMachine);

The OOTBAbstractPersistingStateMachineInterceptor implementation persists mostly inpreStateChange method. What I observed is that for thepostStateChange method,stateMachine androotStateMachine for regions are always correct and different objects.

However, forpreStateChange thestateMachine androotStateMachine are always (or almost always, I don't recall) the same object. Meaning there is a bug imho somewhere upstream.

Now the "Persisting" issues can be quickly fixed by using custom persister and persisting onpostStateChange only. HoweverResuming issues need to be fixed in the library/project itself.

So, I have two questions... do you think we can split the fixes for persisting & resuming? How can I help to push these things through? I can gladly take over.

yes there is a bug AbstractStateMachine which is calling AbstractPersistingStateMachineInterceptor with incorrect state machine. can we prioritise this to fix in the library? we fixed some parts by extending the library. however AbstractStateMachine is so complicated as there are no proper extension points