fixes #5900 -- address race condition with initialization and site.py loading#5903
Conversation
|
Grrr,
|
fc545a6 to
d90ce9f
Compare
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks for fixing! It would be nice if Py_Initialize were thread-safe, but I think in practice that's unlikely to happen because in real applications there's more obviously a "main thread" which becomes the Python main thread, this auto-initialize behaviour is most commonly seen in tests.
I think I'd prefer to avoid bumping MSRV and go down the call() route. I suspect that we could keep the complexity almost entirely inside interpreter_lifecycle.rs enough that the footgun is less of an issue. We could also document that we should migrate to wait when MSRV is next bumped.
Before rolling back - what is the current MSRV of cryptography? We still have the constraint that Debian trixie is Rust 1.85, so unless we believe that bumping past that is a non-issue, I think we will need to keep MSRV below 1.85 for some time yet. 🤔
|
Currently cryptography is at 1.83. Looked like about ~3.5% of our users where >=1.83 but <1.86. |
| // Py_IsInitialized() can return 1 while Py_InitializeEx is still | ||
| // running (e.g. importing site.py). Block until any in-progress PyO3 |
There was a problem hiding this comment.
This is accurate as implemented in CPython today. (even if we fixed this, older interpreters would still have the behavior)
|
(I switched over to the call version so we don't need to raise MSRV + an assert so if someone mis-used the function it'd trigger.) |
Co-authored-by: David Hewitt <mail@davidhewitt.dev>
|
I merged my cpython PR to fix the Py_Initialize() internals, it should be in the next 3.15 pre-release. |
No description provided.