Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segmentation vault keyboard_client when used in superstate #119

Open
dave992 opened this issue Sep 1, 2023 · 4 comments
Open

Segmentation vault keyboard_client when used in superstate #119

dave992 opened this issue Sep 1, 2023 · 4 comments

Comments

@dave992
Copy link

dave992 commented Sep 1, 2023

Description

I am consistently getting segmentation faults when using the keyboard client and the CbDefaultKeyboardBehavior when its started in a super state.

Overview

I have the following setup running (simplified for this issue):

  • SuperState1
    • InnerState1
    • InnerState2
    • InnerState3
  • NextState1
  • NextState2
  • ErrorState

SuperState1

In SuperState1 I defined a transition to be able to cancel the super state using the keyboard_client as well as a transition from the last inner state to NextState1. This means I had the following transition table:

typedef mpl::list<
  Transition<EvCbSuccess<CbInnerState3, OrReconstruction>, NextState1, SUCCESS>,
  Transition<EvKeyPressX<CbDefaultKeyboardBehavior, OrKeyboard>, ErrorState, ABORT>>
reactions;

And as we need the keyboard client behavior to create the transition events I added the following staticConfigure definition for SuperState1:

static void staticConfigure()
{
  configure_orthogonal<OrKeyboard, CbDefaultKeyboardBehavior>();
}

SuperState1 automatically starts InnerState1 which transitions to InnerState2 after completing a custom client behavior (not related to the keyboard client)

InnerState2

In InnerState2 I wait for user confirmation before transitioning to InnerState3. Initially I had the following transition table and staticConfigure definition:

// TRANSITION TABLE
typedef mpl::list<
  Transition<EvKeyPressN<CbDefaultKeyboardBehavior, OrKeyboard>, StiStopReconstruction, SUCCESS>
>reactions;

// STATE FUNCTIONS
static void staticConfigure()
{
  configure_orthogonal<OrKeyboard, CbDefaultKeyboardBehavior>();
}

After the transition to InnerState3 we automatically move to NextState1 as defined in SuperState1.

NextState1

I again wait for user confirmation in this state before moving to the next state. Resulting in the following transition table and staticConfigure definition:

// TRANSITION TABLE
typedef mpl::list<
  Transition<EvKeyPressU<CbDefaultKeyboardBehavior, OrKeyboard>, NextState2, SUCCESS>
>reactions;

// STATE FUNCTIONS
static void staticConfigure()
{
  configure_orthogonal<OrKeyboard, CbDefaultKeyboardBehavior>();
}

Once I try to trigger the state transition to NextState2 by pressing u I consistently receive a segmentation fault on:

Observed behavior

I noticed that after InnerState2 was started the keyboard client would send 1 log message to my terminal while I would receive 2 log messages from the client behavior(s). My first thought was that this could trigger the transition to NextState1 twice and maybe cause the segmentation fault during the second trigger.

  • Is this actually possible? I thought it might be as the transition table does not contain any info that allows it to see from which copy of the client behavior the event was received. In other words, it is not able to see the difference from the client behavior created in its own state vs the one created in the super state.

I tried the following to resolve this:

Attempt 1

Not starting the keyboard client behavior in InnerState2 by commenting out the configure_orthogonal<OrKeyboard, CbDefaultKeyboardBehavior>(); This still resulted in the segmentation fault.

My rationale was that I do not need it in here if it is still active from SuperState1 and would therefore not result in possibly two transitions being triggered. After this change, I no longer received 2 log messages from the client behaviors but only 1 as expected.

Attempt 2

Not starting the keyboard client behavior in the SuperState1 by commenting out the configure_orthogonal<OrKeyboard, CbDefaultKeyboardBehavior>();. This no longer results in any segmentation faults.

This would however also mean that during InnerState1 and InnerState3 I have no means of canceling unless I also add the client behaviors there.

┆Issue is synchronized with this Jira Task by Unito

@dave992
Copy link
Author

dave992 commented Sep 1, 2023

Is it actually possible to trigger a transition twice by having two client behaviors (one in a super state and one in a inner state)? I thought it might be because the transition table does not contain any info that allows it to see from which copy of the client behavior the event was received. In other words, it is not able to see the difference between the client behavior created in its own state vs. the one created in the super state.

@yassiezar
Copy link
Contributor

Hi @dave992, I've run into a similar issue before with modestates and inner states not behaving properly and occasionally segfaulting. In my case, it was a race condition and didn't crash reliably (you can see my attempted fix in #88 in case it helps) - do you run into guaranteed crashes in your case? If so, you might have a different issue than I did.

I'd say that you shouldn't have 2 of the same transition-triggering CBs active in the same state as you did (the CBs running in a parent state will also run in the child) . As you rightly point out, the transitioning mechanism doesn't use information from the owing state - rather, it loops over the active CBs and takes the first one of the correct type to determine the transition event that's needed. This likely won't cause issues, because the CB attached to the child state being transitioned from will be deleted, while the CB in the parent state will remain active, which is presumably the desired behaviour.

Is it actually possible to trigger a transition twice by having two client behaviors (one in a super state and one in a inner state)

I don't think 2 transitions can be triggered at the same time. As soon as one transition event is fired, the state machine is locked and the transition is executed, preventing all other transition events from being processed (see the flag check here)

@dave992
Copy link
Author

dave992 commented Sep 13, 2023

I am quite certain that it is a race condition too.

A large majority of the segfaults occur on the postEventKeyPress function, but I have encountered some cases where it crashes one or two function calls later in the call stack. Very rarely I do not encounter any error, but during the debugging of this issue this only happened once or twice.

I'd say that you shouldn't have 2 of the same transition-triggering CBs active in the same state as you did (the CBs running in a parent state will also run in the child) .

Looking back on it this indeed does not make too much sense. When I implemented it, I incorrectly assumed the CB and the events were tied specifically to the state it was started in. During the debugging of this issue, I found this not to be the case.

Ideally, I would just run the CB in the superstate, but I still run into the race condition then. So for now I just start the client behavior only from the inner states.

@yassiezar
Copy link
Contributor

@dave992 ok it sounds like you maybe ran into the same problem I did. My fix in #88 worked for me, maybe it helps you? Let me know if it does and I'll push for it to get merged in.

The problem seems to be that, in cases where you add a CB in a parent state, during the state entry phase where all the CBs are initialised, the flag that prevents a CB from being added in both the child and the parent states isn't being set on time (its set a different thread, hence the race condition). The solution is to lock the state machine, move set the TRANSITIONING flag sooner and only add the CB in the parent state (where its meant to go)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants