fix race with sys_rt_exec_state by breaking out bools - v2#740
Open
treib wants to merge 1 commit intobdring:Devtfrom
Open
fix race with sys_rt_exec_state by breaking out bools - v2#740treib wants to merge 1 commit intobdring:Devtfrom
treib wants to merge 1 commit intobdring:Devtfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A simple fix for the race condition with
sys_rt_exec_statethat breaks out all the state bits into separate bools.A few notes:
I renamed the
cycle_stopglobal 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 asys_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 thecycle_stopbit 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 inprotocol_exec_rt_system()for the if/else logic to generate the next state.