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

avoid calling PyType_GetSlot on static types before Python 3.10 #4599

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

davidhewitt
Copy link
Member

Split from #4563

I ran into a crash trying to merge that PR because I ended up calling PyType_GetSlot on a static type, which isn't legal before Python 3.10.

This led to a problem... what to do about abi3 on Python 3.7 through 3.9, where we do occasionally need to look up slot in type objects, but the "good" way of using PyType_GetSlot may cause crashes. The PyTypeObject definition is also deliberately not available in the limited API.

The only practical workaround I could think of is to inline the PyTypeObject definition (as of 3.9) into a helper function .get_slot(SLOT_NAME), which tries to do the right thing in each of the three cases:

  • unlimited api: look in the type object
  • abi3 and runtime version above 3.10: use PyType_GetSlot
  • abi3 and runtime version below 3.10: look in the 3.9 snapshot of the type object

@davidhewitt
Copy link
Member Author

cc @alex as a consumer of abi3, is this workaround acceptable to you?

@alex
Copy link
Contributor

alex commented Oct 4, 2024

Hmm, so the basic assumption here is that it's ok to hardcode some structs (that are not ABI3) because we're only doing it for old versions, and obviously those old versions can't change retroactively?

@davidhewitt
Copy link
Member Author

Yes, with a check at runtime to be sure we aren't doing this on newer python versions.

@alex
Copy link
Contributor

alex commented Oct 4, 2024 via email

@davidhewitt
Copy link
Member Author

I think for the original uses you implemented, it was only called for our heap types. But it was tempting enough for me to copy the same pattern and use it on arbitrary types... kaboom 🤣

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