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

fix race with sys_rt_exec_state by breaking out bools - v2 #740

Open
wants to merge 1 commit into
base: Devt
Choose a base branch
from

Conversation

treib
Copy link

@treib treib commented Jan 17, 2021

A simple fix for the race condition with sys_rt_exec_state that breaks out all the state bits into separate bools.

A few notes:

  • I renamed the cycle_stop global that was broken out previously to be consistent with the newly-added bits.

  • In the existing code, in three places (the main protocol_exec_rt_system() as well as the limit check loops in Limits.cpp and CoreXY.cpp) the ExecState is captured into a local copy which is then used in the if/else logic. To keep the functionality and structure the same, I made a sys_get_rt_exec_state() call in System.cpp that pulls all the global bools back into a bitfield for this purpose. This all should be functionally the same as before.

    Note that sys_get_rt_exec_state() includes the cycle_stop bit as it used to in the olden days, and I changed the if/else to check it that way for consistency. (This may make line 275 in Protocol.cpp look like a mistake at first glance, but it should be correct.)

  • Previously there was some discussion on whether sys_get_rt_exec_state() and the ExecState bitfield are necessary, but I feel it is a clean way to snapshot the current state in protocol_exec_rt_system() for the if/else logic to generate the next state.

@MitchBradley MitchBradley changed the base branch from Devt to DevtSane February 18, 2021 21:34
Base automatically changed from DevtSane to Devt February 18, 2021 21:47
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

Successfully merging this pull request may close these issues.

1 participant