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 baseband thread init order bug for all procs. #1293

Merged
merged 1 commit into from Jul 23, 2023

Conversation

kallanreed
Copy link
Contributor

This change should hopefully address all possible baseband processor initialization race conditions.
In C++ the in-body initializers are run in declaration order. This means that the baseband_thread will start running immediately upon its declaration. This means that it will start sending data to the processor while the processor itself is still initializing which can cause all kinds of undefined behavior.

This change has two main parts.

  1. Move the thread declarations to the ends of all the classes so the processor will be otherwise initialized when the baseband_thread starts.
  2. Some processors have custom constructor definitions to set up additional member state. For those, it's best to delay the baseband_thread start until the end of the constructor. To support this, I added a "start" method to ThreadBase and decoupled initialization from start. I changed the thread constructor to make the defaults behave like it had previously.

Copy link
Member

@gullradriel gullradriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good to me. Let's merge and widen the range of testers

@gullradriel gullradriel merged commit 7bd370b into portapack-mayhem:next Jul 23, 2023
3 checks passed
@kallanreed kallanreed deleted the threading_fix branch July 25, 2023 14:40
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.

None yet

2 participants