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

Remove undefined behavior from the startup conditions #132

Merged
merged 3 commits into from
Sep 24, 2024

Conversation

mxgrey
Copy link
Collaborator

@mxgrey mxgrey commented Sep 23, 2024

I noticed a very non-deterministic behavior where sometimes one or more lifts would be unable to function from the moment the simulation started, and nothing seemed to allow them to recover. This would only happen once in a while and there was no consistency except that it happened immediately at startup and not under any other circumstance.

After some debugging I found that the DoorModeCmp for lifts was being initialized without a specific value set for the door_state. Most of the time memory used by programs will be zero-initialized, so most of the time the door_state field was getting set to 0, which coincidentally gives us the correct behavior. But a zero-initialization is not guaranteed, so when that doesn't happen the door_state field will be some arbitrary number, and then the state machine for the lift will just never be functional.

This PR simply makes sure that field is set to the correct value at initialization. I haven't seen the bug happen again after this fix.

Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
@luca-della-vedova luca-della-vedova merged commit 0a409cb into main Sep 24, 2024
5 checks passed
@luca-della-vedova luca-della-vedova deleted the remove_undefined_behavior branch September 24, 2024 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants