fix: Python 3.13t with GIL (pybind#5139)

* ci: try Python 3.13t Signed-off-by: Henry Schreiner <[email protected]> * fix: support Python 3.13t Signed-off-by: Henry Schreiner <[email protected]> * fix: patch PyPy Signed-off-by: Henry Schreiner <[email protected]> * tests: one more int cast Signed-off-by: Henry Schreiner <[email protected]> * tests: cleanup Signed-off-by: Henry Schreiner <[email protected]> * refactor: use named constant in tests for immortal refcounts Signed-off-by: Henry Schreiner <[email protected]> * docs: move comment about free threaded Python Signed-off-by: Henry Schreiner <[email protected]> --------- Signed-off-by: Henry Schreiner <[email protected]>
anchao · May 28, 2024 · ae6432b · ae6432b
1 parent a5b9e50
commit ae6432b
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 9 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -196,6 +196,35 @@ jobs:
         pytest tests/extra_setuptools
       if: "!(matrix.runs-on == 'windows-2022')"
 
+  manylinux:
+    name: Manylinux on 🐍 3.13t • GIL
+    runs-on: ubuntu-latest
+    timeout-minutes: 40
+    container: quay.io/pypa/musllinux_1_2_x86_64:latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Prepare venv
+        run: python3.13 -m venv .venv
+
+      - name: Install Python deps
+        run: .venv/bin/pip install -r tests/requirements.txt
+
+      - name: Configure C++11
+        run: >
+          cmake -S. -Bbuild
+          -DPYBIND11_WERROR=ON
+          -DDOWNLOAD_CATCH=ON
+          -DDOWNLOAD_EIGEN=ON
+          -DPython_ROOT_DIR=.venv
+
+      - name: Build C++11
+        run: cmake --build build -j2
+
+      - name: Python tests C++11
+        run: cmake --build build --target pytest -j2
 
   deadsnakes:
     strategy:

diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
@@ -183,7 +183,15 @@ class object_api : public pyobject_tag {
     str_attr_accessor doc() const;
 
     /// Return the object's current reference count
-    int ref_count() const { return static_cast<int>(Py_REFCNT(derived().ptr())); }
+    ssize_t ref_count() const {
+#ifdef PYPY_VERSION
+        // PyPy uses the top few bits for REFCNT_FROM_PYPY & REFCNT_FROM_PYPY_LIGHT
+        // Following pybind11 2.12.1 and older behavior and removing this part
+        return static_cast<ssize_t>(static_cast<int>(Py_REFCNT(derived().ptr())));
+#else
+        return Py_REFCNT(derived().ptr());
+#endif
+    }
 
     // TODO PYBIND11_DEPRECATED(
     //     "Call py::type::handle_of(h) or py::type::of(h) instead of h.get_type()")

diff --git a/tests/pybind11_tests.cpp b/tests/pybind11_tests.cpp
@@ -89,6 +89,8 @@ PYBIND11_MODULE(pybind11_tests, m) {
 #endif
     m.attr("cpp_std") = cpp_std();
     m.attr("PYBIND11_INTERNALS_ID") = PYBIND11_INTERNALS_ID;
+    // Free threaded Python uses UINT32_MAX for immortal objects.
+    m.attr("PYBIND11_REFCNT_IMMORTAL") = UINT32_MAX;
     m.attr("PYBIND11_SIMPLE_GIL_MANAGEMENT") =
 #if defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
         true;

diff --git a/tests/test_class.py b/tests/test_class.py
@@ -3,7 +3,7 @@
 import pytest
 
 import env
-from pybind11_tests import ConstructorStats, UserType
+from pybind11_tests import PYBIND11_REFCNT_IMMORTAL, ConstructorStats, UserType
 from pybind11_tests import class_ as m
 
 
@@ -377,7 +377,9 @@ class PyDog(m.Dog):
         refcount_3 = getrefcount(cls)
 
         assert refcount_1 == refcount_3
-        assert refcount_2 > refcount_1
+        assert (refcount_2 > refcount_1) or (
+            refcount_2 == refcount_1 == PYBIND11_REFCNT_IMMORTAL
+        )
 
 
 def test_reentrant_implicit_conversion_failure(msg):

diff --git a/tests/test_embed/CMakeLists.txt b/tests/test_embed/CMakeLists.txt
@@ -7,6 +7,13 @@ if("${PYTHON_MODULE_EXTENSION}" MATCHES "pypy" OR "${Python_INTERPRETER_ID}" STR
   return()
 endif()
 
+if(TARGET Python::Module AND NOT TARGET Python::Python)
+  message(STATUS "Skipping embed test since no embed libs found")
+  add_custom_target(cpptest) # Dummy target since embedding is not supported.
+  set(_suppress_unused_variable_warning "${DOWNLOAD_CATCH}")
+  return()
+endif()
+
 find_package(Catch 2.13.9)
 
 if(CATCH_FOUND)

diff --git a/tests/test_kwargs_and_defaults.cpp b/tests/test_kwargs_and_defaults.cpp
@@ -150,10 +150,13 @@ TEST_SUBMODULE(kwargs_and_defaults, m) {
 
 // test_args_refcount
 // PyPy needs a garbage collection to get the reference count values to match CPython's behaviour
+// PyPy uses the top few bits for REFCNT_FROM_PYPY & REFCNT_FROM_PYPY_LIGHT, so truncate
 #ifdef PYPY_VERSION
 #    define GC_IF_NEEDED ConstructorStats::gc()
+#    define REFCNT(x) (int) Py_REFCNT(x)
 #else
 #    define GC_IF_NEEDED
+#    define REFCNT(x) Py_REFCNT(x)
 #endif
     m.def("arg_refcount_h", [](py::handle h) {
         GC_IF_NEEDED;
@@ -172,7 +175,7 @@ TEST_SUBMODULE(kwargs_and_defaults, m) {
         py::tuple t(a.size());
         for (size_t i = 0; i < a.size(); i++) {
             // Use raw Python API here to avoid an extra, intermediate incref on the tuple item:
-            t[i] = (int) Py_REFCNT(PyTuple_GET_ITEM(a.ptr(), static_cast<py::ssize_t>(i)));
+            t[i] = REFCNT(PyTuple_GET_ITEM(a.ptr(), static_cast<py::ssize_t>(i)));
         }
         return t;
     });
@@ -182,7 +185,7 @@ TEST_SUBMODULE(kwargs_and_defaults, m) {
         t[0] = o.ref_count();
         for (size_t i = 0; i < a.size(); i++) {
             // Use raw Python API here to avoid an extra, intermediate incref on the tuple item:
-            t[i + 1] = (int) Py_REFCNT(PyTuple_GET_ITEM(a.ptr(), static_cast<py::ssize_t>(i)));
+            t[i + 1] = REFCNT(PyTuple_GET_ITEM(a.ptr(), static_cast<py::ssize_t>(i)));
         }
         return t;
     });

diff --git a/tests/test_kwargs_and_defaults.py b/tests/test_kwargs_and_defaults.py
@@ -1,5 +1,6 @@
 import pytest
 
+from pybind11_tests import PYBIND11_REFCNT_IMMORTAL
 from pybind11_tests import kwargs_and_defaults as m
 
 
@@ -384,7 +385,7 @@ def test_args_refcount():
     myval = 54321
     expected = refcount(myval)
     assert m.arg_refcount_h(myval) == expected
-    assert m.arg_refcount_o(myval) == expected + 1
+    assert m.arg_refcount_o(myval) in {expected + 1, PYBIND11_REFCNT_IMMORTAL}
     assert m.arg_refcount_h(myval) == expected
     assert refcount(myval) == expected
 
@@ -420,6 +421,7 @@ def test_args_refcount():
     # for the `py::args`; in the previous case, we could simply inc_ref and pass on Python's input
     # tuple without having to inc_ref the individual elements, but here we can't, hence the extra
     # refs.
-    assert m.mixed_args_refcount(myval, myval, myval) == (exp3 + 3, exp3 + 3, exp3 + 3)
+    exp3_3 = PYBIND11_REFCNT_IMMORTAL if exp3 == PYBIND11_REFCNT_IMMORTAL else exp3 + 3
+    assert m.mixed_args_refcount(myval, myval, myval) == (exp3_3, exp3_3, exp3_3)
 
     assert m.class_default_argument() == "<class 'decimal.Decimal'>"
diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
@@ -5,7 +5,7 @@
 import pytest
 
 import env
-from pybind11_tests import detailed_error_messages_enabled
+from pybind11_tests import PYBIND11_REFCNT_IMMORTAL, detailed_error_messages_enabled
 from pybind11_tests import pytypes as m
 
 
@@ -635,7 +635,7 @@ def test_memoryview_refcount(method):
     ref_before = sys.getrefcount(buf)
     view = method(buf)
     ref_after = sys.getrefcount(buf)
-    assert ref_before < ref_after
+    assert ref_before < ref_after or ref_before == ref_after == PYBIND11_REFCNT_IMMORTAL
     assert list(view) == list(buf)