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

[BUG]: Incompatible internals between versions #5420

Open
2 of 3 tasks
rostan-t opened this issue Oct 24, 2024 · 7 comments
Open
2 of 3 tasks

[BUG]: Incompatible internals between versions #5420

rostan-t opened this issue Oct 24, 2024 · 7 comments
Labels
triage New bug, unverified

Comments

@rostan-t
Copy link

Required prerequisites

What version (or hash if on master) of pybind11 are you using?

v2.13.6

Problem description

Version 2.13.2 replaces std::mutex with PyMutex in struct internals (#5219) in free-threaded builds. This changes the size and alignment of fields in internals because std::mutex occupies 40 bytes on x86-64 Linux while PyMutex only requires 1.

This can lead to crashes if same program imports two libraries compiled resp. with pybind11 before and after 2.13.2. When the first library is imported, its first call to detail::get_internals() will find that the interpreter state dict doesn't yet contain an internal object, initialize one and store it (internals.h:541). When the next library is imported, it will load the object from the state dict and cast the void * to internals *. At this point, no error occurs but the object is already corrupted. When the second library tries to use the cached internals object, unexpected things such as segfaults will happen.

A fix for this would be to put the mutex at the end of the structure.

Reproducible example code

- Compile module foo with pybind v2.13.1
- Compile module bar with pybind v2.13.2

import foo
import bar

Is this a regression? Put the last known working version here if it is.

v2.13.1

@rostan-t rostan-t added the triage New bug, unverified label Oct 24, 2024
@colesbury
Copy link
Contributor

Oops - I should have bumped the version number in the PyMutex PR.

@vfdev-5 already bumped the version number in the master branch, but it's not in the v2.13 branch:

# ifdef Py_GIL_DISABLED
# define PYBIND11_INTERNALS_VERSION 6
# else

I'm not sure there's much to be done other than "don't use v2.13.1" with free-threaded builds. @rostan-t, are you aware of any free-threading compatible libraries that are pinned to v2.13.1?

@vfdev-5
Copy link
Contributor

vfdev-5 commented Oct 24, 2024

Scipy is FT compatible and pinning pybind11 as "pybind11>=2.13.1":
https://github.com/scipy/scipy/blob/59fd061b353881ad6cf84e3fd9aa398de8ecb136/pyproject.toml#L22

@rostan-t
Copy link
Author

ContourPy also pins pybind11 >= 2.13.1.

@colesbury
Copy link
Contributor

Is pybind11 >= 2.13.1 a problem or just pybind11 == 2.13.1? Won't those packages build with v2.13.6 now that it's released? @rostan-t, did you encounter this problem with a specific package?

@ianthomas23
Copy link

ContourPy is fine. The latest release 1.3.0 (the only release that supports Python 3.13) was built using pybind11 2.13.5 on PyPI and 2.13.6 on conda-forge, confirmed using:

python -c "import contourpy.util as u; print(u.build_config()['pybind11_version'])"

I'll change the pin to pybind11 >= 2.13.2 anyway.

@rgommers
Copy link
Contributor

SciPy also shouldn't be an issue; the 1.14.x branch doesn't support free-threading and has an upper bound of pybind11<2.13.0, while the nightly wheels are built with latest pybind11 (last uploaded ones were built with pybind11 2.13.6).

I'll change the pin to pybind11 >= 2.13.2 anyway.

PR has been merged in SciPy with the same change.

@ianthomas23
Copy link

Matplotlib 3.9.2 might be an issue. The cp313t PyPI wheels were built on the same day that pybind11 2.13.2 and 2.13.3 were released so they may have been built using pybind11 2.13.1, I don't know yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage New bug, unverified
Projects
None yet
Development

No branches or pull requests

5 participants