Bug 2298403 - Patch request: unsafe subinterpreter support (needed to unbreak ceph)
Summary: Patch request: unsafe subinterpreter support (needed to unbreak ceph)
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: rust-pyo3
Version: rawhide
Hardware: aarch64
OS: Linux
unspecified
high
Target Milestone: ---
Assignee: Rust SIG
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 2255688 2298404
TreeView+ depends on / blocked
 
Reported: 2024-07-17 06:03 UTC by Hector Martin
Modified: 2024-08-11 10:31 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2024-08-11 10:31:10 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Hector Martin 2024-07-17 06:03:06 UTC
python3-bcrypt 4.x switched to using rust-pyo3. rust-pyo3 does not support subinterpreters yet. This broke Ceph, which uses python3-bcrypt and subinterpreters. The story here is further complicated by there having been multiple versions of the subinterpreter check in pyo3, leading older versions to "work" by accident.

Upstream is working on proper subinterpreter support, but right *now* ceph-mgr is completely broken in f39, f40, and rawhide, and has been for many months. We really need a solution for this now, not later.

Please add this patch to rust-pyo3:
https://git.st8l.com/luxolus/pyo3/commit/338c71d0ad10f7ae38b7b44e576d49b91ed20d99

This patch essentially disables the subinterpreter safety check if built with a new feature flag `unsafe-allow-subinterpreters`. It has been determined that in the case of python3-bcrypt specifically, it is actually safe to do this. If this is backported and `python3-bcrypt` switches on that feature, then Ceph will be fixed.

Reproducible: Always

Comment 1 Hector Martin 2024-07-17 06:16:02 UTC
FTR, python3-bcrypt uses PyO3 0.19 at this time in F39/F40: https://src.fedoraproject.org/rpms/python-bcrypt/tree/f40

And 0.20 in rawhide (which is on python3-bcrypt 4.1.2): https://github.com/pyca/bcrypt/blob/4.1.2/src/_bcrypt/Cargo.toml

So those are the versions that would need the patch to allow python3-bcrypt to work (after it turns on the feature) and unbreak ceph.

Comment 2 Neal Gompa 2024-07-17 12:23:49 UTC
Since this affects every version of Fedora right now, I've set this to "rawhide".

Comment 3 Fabio Valentini 2024-07-19 19:35:38 UTC
> Please add this patch to rust-pyo3:
> https://git.st8l.com/luxolus/pyo3/commit/338c71d0ad10f7ae38b7b44e576d49b91ed20d99

What this patch does not not really an acceptable solution for Fedora packaging.
I will do something that should be equivalent for what's needed to "fix" bcrypt / whatever component of ceph loads the bcrypt module.

The difference will likely be that in python-bcrypt an environment variable needs to be exported during build instead of patching in a feature flag for pyo3.

Note that python-bcrypt really should bump its pyo3 dependency to at least v0.21. Any older versions are slightly broken, and the only version of pyo3 that officially supporty python 3.13 (without my backports for pyo3 packages in Fedora) is v0.22.

Comment 4 Hector Martin 2024-07-23 11:32:31 UTC
Whatever works, as long as the end result is equivalent :)

Thanks in advance for taking care of this (it's been broken for ages now).

Comment 5 Fabio Valentini 2024-07-23 13:42:11 UTC
Progress update: I made a slightly different patch for pyo3. The usual way to enable "experimental" or "unsafe" things in Rust crates is via "cfg" flags, not via feature flags (for example, the "tokio_unstable" flag for tokio). The cfg flag I added for pyo3 for this use case is "pyo3_unsafe_allow_subinterpreters".

So the changed subinterpreter check can be toggled by adding "--cfg pyo3_unsafe_allow_subinterpreters" to the RUSTFLAGS when building something. Adding this to the RUSTFLAGS environment variable (i.e. *appending*, but *NOT* overriding the existing value) in the %build script for python-bcrypt should do what's needed to fix bcrypt for cephmgr.

Builds of pyo3 0.22.2 (update from 0.22.0), pyo3 0.21, and pyo3 0.20 are running right now, and will be available in rawhide soon.

However, backporting this to pyo3 0.19 does not look possible (or at least, not without investing significantly more work), because the patch does not backport cleanly to pyo3 0.19. Looks like there is no subinterpreter check in 0.19 at all, so I'm not sure what would need to be done there.

So ideally, python-bcrypt should be patched to use a newer version of pyo3 instead. Relying on old versions of pyo3 is a bad idea, since compatibility with new CPython versions is not guaranteed. For example, I backported minimal Python 3.13 support to all currently packaged pyo3 versions, but that's not guaranteed to work at all, since it's not tested or supported by PyO3 upstream.

Comment 6 Fabio Valentini 2024-07-23 14:55:50 UTC
The updates are available now:
https://bodhi.fedoraproject.org/updates/?search=rust-pyo3-0.22.2

Please let me know if this works for you.

Comment 7 Hector Martin 2024-07-26 13:22:15 UTC
pyo3 0.19 does have a subinterpreter check, but it was different: https://github.com/PyO3/pyo3/blob/release-0.19/src/impl_/pymodule.rs#L77

I'll try to get python-bcrypt updated and test the change.

Comment 8 Hector Martin 2024-07-26 13:49:01 UTC
Jumping all the way to 0.22 doesn't seem that easy since python-bcrypt needs changes for that. But jumping to 0.20 does work without any further patching, and together with the --cfg does the trick and makes ceph-mgr work again :)

I'll send a PR to python-bcrypt for this.

Comment 9 Hector Martin 2024-08-11 10:31:10 UTC
I think we can close this now, thanks!


Note You need to log in before you can comment on or make changes to this bug.