Skip to content

timers: Add some locking#436

Merged
chrissie-c merged 1 commit intoClusterLabs:masterfrom
chrissie-c:fix-timer-locking
Feb 8, 2021
Merged

timers: Add some locking#436
chrissie-c merged 1 commit intoClusterLabs:masterfrom
chrissie-c:fix-timer-locking

Conversation

@chrissie-c
Copy link
Copy Markdown
Contributor

Fix several locking issues reported by helgrind

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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