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.

Sign up for GitHub

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

Jump to bottom

[C API] Py_MOD_PER_INTERPRETER_GIL_SUPPORTED added to limited C API without versionning #110968

Closed
vstinner opened this issue Oct 17, 2023 · 4 comments
Closed

[C API] Py_MOD_PER_INTERPRETER_GIL_SUPPORTED added to limited C API without versionning #110968

vstinner opened this issue Oct 17, 2023 · 4 comments
Labels
topic-C-API

Comments

@vstinner
Copy link
Member

vstinner commented Oct 17, 2023 •
edited by bedevere-app bot
Loading

Three constants for PyModuleDef_Slot were added to the limited C API in Python 3.13:

  • Py_MOD_MULTIPLE_INTERPRETERS_NOT_SUPPORTED
  • Py_MOD_MULTIPLE_INTERPRETERS_SUPPORTED
  • Py_MOD_PER_INTERPRETER_GIL_SUPPORTED

Problem: there are added without version, as if they are available on Python 3.12 and older, whereas it's not the case.

cc @encukou @ericsnowcurrently

Linked PRs
vstinner added a commit to vstinner/cpython that referenced this issue Oct 17, 2023
@vstinner
pythongh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED new in 3.13.
00e0e4e 
* Only add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED to limited C API
  version 3.13.
* errno, xxlimited and _ctypes_test extensions now need the limited C
  API version 3.13 to get Py_MOD_PER_INTERPRETER_GIL_SUPPORTED.  They
  now include standard header files explicitly: <errno.h>, <string.h>
  and <stdio.h>.
* xxlimited_35: Remove Py_mod_multiple_interpreters slot,
  incompatible with limited C API version 3.5.
vstinner added a commit to vstinner/cpython that referenced this issue Oct 17, 2023
@vstinner
pythongh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED new in 3.13.
8a29b98 
* Only add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED to limited C API
  version 3.13.
* errno, xxlimited and _ctypes_test extensions now need the limited C
  API version 3.13 to get Py_MOD_PER_INTERPRETER_GIL_SUPPORTED.  They
  now include standard header files explicitly: <errno.h>, <string.h>
  and <stdio.h>.
* xxlimited_35: Remove Py_mod_multiple_interpreters slot,
  incompatible with limited C API version 3.5.
@bedevere-app bedevere-app bot mentioned this issue Oct 17, 2023
Merged
vstinner added a commit that referenced this issue Oct 17, 2023
@vstinner
gh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED new in 3.13. (#110969)
db15fc2 
* Only add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED to limited C API
  version 3.13.
* errno, xxlimited and _ctypes_test extensions now need the limited C
  API version 3.13 to get Py_MOD_PER_INTERPRETER_GIL_SUPPORTED.  They
  now include standard header files explicitly: <errno.h>, <string.h>
  and <stdio.h>.
* xxlimited_35: Remove Py_mod_multiple_interpreters slot,
  incompatible with limited C API version 3.5.
@encukou
Copy link
Member

encukou commented Oct 17, 2023

IMO it's harmless to include these, even if they'd just be treated as false/true on older versions.

The bigger problem here is the slot ID, Py_mod_multiple_interpreters. It'll make the extension fail to load if used with an older version. That should definitely be in an #ifdef.

@ericsnowcurrently
Copy link
Member

Yeah, it was definitely unintentional to expose any of these in the wrong place.

Also, they were added to 3.12 (1c420e1), right before feature freeze. Would we be okay to add the version ifdefs in 3.12.1? (I'd think so, but I sometimes get this subtle sort of compatibility concern wrong.)

The bigger problem here is the slot ID, Py_mod_multiple_interpreters. It'll make the extension fail to load if used with an older version. That should definitely be in an #ifdef.

Agreed. gh-110969 missed fixing it.

vstinner added a commit to vstinner/cpython that referenced this issue Nov 1, 2023
@vstinner
pythongh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED was added to 3.12
35bd711
@bedevere-app bedevere-app bot mentioned this issue Nov 1, 2023
Merged
@vstinner
Copy link
Member Author

vstinner commented Nov 1, 2023

Also, they were added to 3.12 (1c420e1), right before feature freeze. Would we be okay to add the version ifdefs in 3.12.1?

Oh, I missed that. I wrote PR #111584 to fix the version check in the main branch, and then I will backport the change to the 3.12 branch.

vstinner added a commit to vstinner/cpython that referenced this issue Nov 1, 2023
@vstinner
pythongh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED was added to 3.12
cfe5e5a
vstinner added a commit that referenced this issue Nov 1, 2023
@vstinner
gh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED was added to 3.12 (#1…
ef83b3f 
…11584)
@bedevere-app bedevere-app bot mentioned this issue Nov 1, 2023
Merged
vstinner added a commit to vstinner/cpython that referenced this issue Nov 1, 2023
@vstinner
pythongh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED was added to 3.12
a255169 
Constants like Py_MOD_PER_INTERPRETER_GIL_SUPPORTED were only added
to the limited C API version 3.12 and newer.
vstinner added a commit that referenced this issue Nov 1, 2023
@vstinner
[3.12] gh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED was added to 3…
d3d2cfe 
….12 (#111588)

Constants like Py_MOD_PER_INTERPRETER_GIL_SUPPORTED were only added
to the limited C API version 3.12 and newer.
@vstinner vstinner closed this as completed Nov 3, 2023
FullteaR pushed a commit to FullteaR/cpython that referenced this issue Nov 3, 2023
@vstinner @FullteaR
pythongh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED was added to 3.…
ad0067e 
…12 (python#111584)
@ericsnowcurrently
Copy link
Member

Thanks for taking care of this, @vstinner!

This was referenced Nov 3, 2023
Closed
Merged
ericsnowcurrently added a commit that referenced this issue Nov 6, 2023
@ericsnowcurrently
gh-111698: Restrict Py_mod_multiple_interpreters to 3.12+ Under Py_LI…
836e0a7 
…MITED_API (gh-111707)

This should have been done in gh-104148.

(A similar fix has already be done for that slot's value macros, and backported to 3.12.  See gh-110968.)
@miss-islington miss-islington mentioned this issue Nov 6, 2023
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 6, 2023
@ericsnowcurrently @miss-islington
pythongh-111698: Restrict Py_mod_multiple_interpreters to 3.12+ Under…
978fb85 
… Py_LIMITED_API (pythongh-111707)

This should have been done in pythongh-104148.

(A similar fix has already be done for that slot's value macros, and backported to 3.12.  See pythongh-110968.)
(cherry picked from commit 836e0a7)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
hugovk pushed a commit to hugovk/cpython that referenced this issue Nov 8, 2023
@ericsnowcurrently @hugovk
pythongh-111698: Restrict Py_mod_multiple_interpreters to 3.12+ Under…
7af88ed 
… Py_LIMITED_API (pythongh-111707)

This should have been done in pythongh-104148.

(A similar fix has already be done for that slot's value macros, and backported to 3.12.  See pythongh-110968.)
@vstinner vstinner mentioned this issue Nov 13, 2023
Open
ericsnowcurrently added a commit that referenced this issue Nov 28, 2023
@miss-islington @ericsnowcurrently
[3.12] gh-111698: Restrict Py_mod_multiple_interpreters to 3.12+ Unde…
f7251e2 
…r Py_LIMITED_API (gh-111707) (gh-111787)

This should have been done in gh-104148.

(A similar fix has already be done for that slot's value macros, and backported to 3.12.  See gh-110968.)
(cherry picked from commit 836e0a7)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@vstinner @aisk
pythongh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED new in 3.13. (p…
6c40c35 
…ython#110969)

* Only add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED to limited C API
  version 3.13.
* errno, xxlimited and _ctypes_test extensions now need the limited C
  API version 3.13 to get Py_MOD_PER_INTERPRETER_GIL_SUPPORTED.  They
  now include standard header files explicitly: <errno.h>, <string.h>
  and <stdio.h>.
* xxlimited_35: Remove Py_mod_multiple_interpreters slot,
  incompatible with limited C API version 3.5.
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@vstinner @aisk
pythongh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED was added to 3.…
b522701 
…12 (python#111584)
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@ericsnowcurrently @aisk
pythongh-111698: Restrict Py_mod_multiple_interpreters to 3.12+ Under…
7b8129e 
… Py_LIMITED_API (pythongh-111707)

This should have been done in pythongh-104148.

(A similar fix has already be done for that slot's value macros, and backported to 3.12.  See pythongh-110968.)
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@vstinner @Glyphack
pythongh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED new in 3.13. (p…
9427803 
…ython#110969)

* Only add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED to limited C API
  version 3.13.
* errno, xxlimited and _ctypes_test extensions now need the limited C
  API version 3.13 to get Py_MOD_PER_INTERPRETER_GIL_SUPPORTED.  They
  now include standard header files explicitly: <errno.h>, <string.h>
  and <stdio.h>.
* xxlimited_35: Remove Py_mod_multiple_interpreters slot,
  incompatible with limited C API version 3.5.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@vstinner @Glyphack
pythongh-110968: Py_MOD_PER_INTERPRETER_GIL_SUPPORTED was added to 3.…
efc914e 
…12 (python#111584)
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@ericsnowcurrently @Glyphack
pythongh-111698: Restrict Py_mod_multiple_interpreters to 3.12+ Under…
71dc27c 
… Py_LIMITED_API (pythongh-111707)

This should have been done in pythongh-104148.

(A similar fix has already be done for that slot's value macros, and backported to 3.12.  See pythongh-110968.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic-C-API
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@vstinner @encukou @ericsnowcurrently