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

timers: Add some locking #436

Merged
merged 1 commit into from
Feb 8, 2021

Conversation

chrissie-c
Copy link
Contributor

Fix several locking issues reported by helgrind

Copy link
Member

@jfriesse jfriesse left a comment

Choose a reason for hiding this comment

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

Looks great and makes libqb a bit more multi thread friendly so ACK. I would just consider increasing sleep timeout in test to prevent false failures because of clogged CI.

Fix several locking issues reported by helgrind
@chrissie-c chrissie-c merged commit d6e2bd1 into ClusterLabs:master Feb 8, 2021
@chrissie-c
Copy link
Contributor Author

Thanks for the review. merged.

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Oct 11, 2021
https://build.opensuse.org/request/show/924180
by user yan_gao + dimstar_suse
- Update to version 2.0.3+20210303.404adbc (v2.0.3):
- syslog: Add a message-id parameter for messages (gh#ClusterLabs/libqb#433)
- timers: Add some locking (gh#ClusterLabs/libqb#436)
- ipcc: Have a few goes at tidying up after a dead server (gh#ClusterLabs/libqb#434)
- strlcpy: Check for maxlen underflow (gh#ClusterLabs/libqb#432)
- doxygen2man: fix printing of lines starting with '.' (gh#ClusterLabs/libqb#431)
- doxygen2man: ignore all-whitespace brief descriptions (gh#ClusterLabs/libqb#430) (forwarded request 924179 from yan_gao)
@sdake
Copy link

sdake commented Jun 13, 2022

While this patch is technically correct, unless the title was “all” it should be reverted. This sets a precedent this library supports concurrency. It doesn’t, and this small change doesn’t move things any closer, if concurrency supports is a goal.

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.

3 participants