Skip to content

Add a lock to deterministic_PRNG #4455

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

Merged
merged 4 commits into from
Jun 30, 2025

Conversation

tybug
Copy link
Member

@tybug tybug commented Jun 29, 2025

Part of #4451.

To reproduce: pytest -k test_zero_contained_examples --parallel-threads 2.

Pushing this lock down to after the hypothesis.core._hypothesis_global_random is None check would in theory avoid the described issue, and reduce overhead to once per session instead of once per test. But pushing it down could create two Random instances, one of which gets overwritten. This makes me nervous since it's possible for a thread to use the overwritten random before it gets overwritten, which may break determinism assumptions.

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

  • I'd like a test for this
  • Solution seems fine to me as an immediate fix, but I wonder whether we should make the _hypothesis_global_random into a threadlocal - that seems like it might better express the intention that this is "engine-global", to coin a term.

@tybug
Copy link
Member Author

tybug commented Jun 30, 2025

but I wonder whether we should make the _hypothesis_global_random into a threadlocal

nice, I like this a lot!

I don't have a great test for this other than the generic "run hypothesis concurrently in lots of threads", which I have written locally, but won't be able to PR until hypothesis is fully thread safe

@Zac-HD
Copy link
Member

Zac-HD commented Jun 30, 2025

I don't have a great test for this other than the generic "run hypothesis concurrently in lots of threads", which I have written locally, but won't be able to PR until hypothesis is fully thread safe

So long as you're tracking tests to eventually land in our CI, I'm pretty relaxed if those don't pass yet - since you're working on the contract and I'm confident that they will in fact land eventually! If it makes sense we could land some xfail tests as we go, tagged so that we un-xfail them all before declaring thread-safety?

@tybug
Copy link
Member Author

tybug commented Jun 30, 2025

If it makes sense we could land some xfail tests as we go, tagged so that we un-xfail them all before declaring thread-safety?

I'll do this for anything more targeted, but for generic thread safety tests I'm going to iterate on them a bunch so I'll keep those for the end

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

ooh, the thread-local works really nicely here 👍

@tybug tybug merged commit 9d46ce9 into HypothesisWorks:master Jun 30, 2025
178 of 181 checks passed
@tybug tybug deleted the free-threading-global-random-lock branch June 30, 2025 19:04
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