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

msgq: refactor blocking recv for improved robustness and performance #616

Merged
merged 6 commits into from
Jan 18, 2025

Conversation

deanlee
Copy link
Contributor

@deanlee deanlee commented May 29, 2024

Replaced the previous method of installing, saving, and restoring signal handlers with a more streamlined and standard approach using signal masks (pthread_sigmask, sigtimedwait) for blocking receive operations. This update enhances the consistency and reliability of signal handling, aligning with best practices. It also removes the need for msgq_poll, thereby improving the efficiency of the receive function.

Additionally, a new test case, test_receive_interrupts_on_sigint, has been added to confirm and verify the functionality of the updated signal handling.

Since signal blocking is now managed in the C++ code, nogil is used in the Cython code to ensure the GIL is released during the execution of the receive function. This prevents it from impacting other Python threads.

@deanlee deanlee marked this pull request as draft June 26, 2024 10:40
@deanlee deanlee force-pushed the cereal_fix_signal branch 5 times, most recently from dcf5f0b to 040ebb4 Compare August 13, 2024 18:08
@deanlee deanlee changed the title msgq: improve blocking receive msgq: refactor blocking recv for improved robustness and performance Aug 13, 2024
@deanlee deanlee force-pushed the cereal_fix_signal branch from 040ebb4 to d36d04c Compare August 13, 2024 18:36
@deanlee deanlee marked this pull request as ready for review August 13, 2024 18:37
@deanlee deanlee force-pushed the cereal_fix_signal branch from d36d04c to c949a78 Compare August 13, 2024 18:38
@deanlee deanlee force-pushed the cereal_fix_signal branch 6 times, most recently from f1112ff to 2cfd4de Compare August 13, 2024 18:58
msgq/impl_msgq.cc Outdated Show resolved Hide resolved
@sshane sshane force-pushed the cereal_fix_signal branch from 63adf99 to 3fcc9f7 Compare January 18, 2025 03:26
@sshane sshane merged commit 5b3f2cd into commaai:master Jan 18, 2025
16 checks passed
sshane added a commit that referenced this pull request Jan 18, 2025
@sshane
Copy link
Contributor

sshane commented Jan 18, 2025

This doesn't work on Mac. Do you think it's worth implementing a mock sigtimedwait if __APPLE__ to poll in 10ms chunks or similar on Mac?

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.

2 participants