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

update pybind and fix race for bucket init. also stress test #477

Merged
merged 5 commits into from
Feb 10, 2025

Conversation

wjblanke
Copy link
Contributor

@wjblanke wjblanke commented Feb 9, 2025

pybind update from 2.11.1 to 2.13.6 to hopefully fix crash.

Copy link

coveralls-official bot commented Feb 9, 2025

Pull Request Test Coverage Report for Build 13231711755

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 79.06%

Totals Coverage Status
Change from base Build 13206229698: 0.03%
Covered Lines: 3500
Relevant Lines: 4427

💛 - Coveralls

@wjblanke wjblanke requested a review from arvidn February 10, 2025 01:47
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

This is definitely a data race, so a good fix.
More generally, keeping global variables in a shared library is really fishy. including the mutex itself. Sometimes unexpected things happen if libraries are unloaded and reloaded but the data section being preserved. I don't think we would get in trouble for this, it's just smelly.
It would probably be better to compute this table at compile time (or as a separate pre-processing step perhaps).

tests/POSStressTest.cpp Show resolved Hide resolved
tests/POSStressTest.cpp Show resolved Hide resolved
@wjblanke wjblanke merged commit 9bc1d62 into main Feb 10, 2025
55 checks passed
@wjblanke wjblanke deleted the pybindwjb branch February 10, 2025 17:47
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