NOT-FOR-MERGING: branch resolve_pr5486_fallout_without_fix#6016
Closed
rwgk wants to merge 6 commits intopybind:masterfrom
Closed
NOT-FOR-MERGING: branch resolve_pr5486_fallout_without_fix#6016rwgk wants to merge 6 commits intopybind:masterfrom
rwgk wants to merge 6 commits intopybind:masterfrom
Conversation
…bindgh-5976) py::enum_ is the primary trigger for pybindgh-5976 because its constructor creates properties via def_property_static / def_property_readonly_static, which call process_attributes::init on already-initialized function records. Yet none of the existing embedding tests used py::enum_ at all. Add an PYBIND11_EMBEDDED_MODULE with py::enum_ and a test case that imports it, finalize/reinitializes the interpreter, and re-imports it. This exercises the def_property_static code path that was fixed in the preceding commit. Note: on Python 3.14.2 (and likely 3.12+), tp_dealloc_impl is not called during Py_FinalizeEx for function record PyObjects — they simply leak because types are effectively immortalized. As a result, this test cannot trigger the original free()-on-string-literal crash on this Python version. However, it remains valuable as a regression guard: on Python builds where finalization does clean up function records (or if CPython changes this behavior), the test would catch the crash. It also verifies that py::enum_ survives interpreter restart correctly, which was previously untested. Made-with: Cursor
Now that the root cause (free of string literals in def_property_static, pybindgh-5976) is fixed in the previous commit, the py_is_finalizing() guards introduced in pybindgh-6010 are no longer needed: - tp_dealloc_impl: remove early return during finalization (was leaking all function records instead of properly destroying them) - destruct(): remove guard around arg.value.dec_ref() - common.h: remove py_is_finalizing() helper (no remaining callers) The genuine fix from pybindgh-6010 (PyObject_Free + Py_DECREF ordering in tp_dealloc_impl) is retained. Made-with: Cursor
Made-with: Cursor
…r. Also fix clang-tidy error.
( cd /wrk/forked/pybind11/tests && PYTHONPATH=/wrk/bld/pybind11_gcc_v3.14.2_df793163d58_default/lib /wrk/bld/pybind11_gcc_v3.14.2_df793163d58_default/TestVenv/bin/python3 -m pytest test_standalone_enum_module.py )
============================= test session starts ==============================
platform linux -- Python 3.14.2, pytest-9.0.2, pluggy-1.6.0
installed packages of interest: build==1.4.2 numpy==2.4.3 scipy==1.17.1
C++ Info: 13.3.0 C++20 __pybind11_internals_v12_system_libstdcpp_gxx_abi_1xxx_use_cxx11_abi_1__ PYBIND11_SIMPLE_GIL_MANAGEMENT=False
rootdir: /wrk/forked/pybind11/tests
configfile: pytest.ini
plugins: timeout-2.4.0, xdist-3.8.0
collected 1 item
test_standalone_enum_module.py F [100%]
=================================== FAILURES ===================================
________________________ test_enum_import_exit_no_crash ________________________
def test_enum_import_exit_no_crash():
# Modeled after reproducer under issue pybind#5976
> env.check_script_success_in_subprocess(
f"""
import sys
sys.path.insert(0, {os.path.dirname(env.__file__)!r})
import standalone_enum_module as m
assert m.SomeEnum.__class__.__name__ == "pybind11_type"
""",
rerun=1,
)
test_standalone_enum_module.py:10:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
code = 'import sys\nsys.path.insert(0, \'/wrk/forked/pybind11/tests\')\nimport standalone_enum_module as m\nassert m.SomeEnum.__class__.__name__ == "pybind11_type"'
def check_script_success_in_subprocess(code: str, *, rerun: int = 8) -> None:
"""Runs the given code in a subprocess."""
import os
import subprocess
import sys
import textwrap
if ANDROID or IOS or sys.platform.startswith("emscripten"):
pytest.skip("Requires subprocess support")
code = textwrap.dedent(code).strip()
try:
for _ in range(rerun): # run flakily failing test multiple times
subprocess.check_output(
[sys.executable, "-c", code],
cwd=os.getcwd(),
stderr=subprocess.STDOUT,
text=True,
)
except subprocess.CalledProcessError as ex:
> raise RuntimeError(
f"Subprocess failed with exit code {ex.returncode}.\n\n"
f"Code:\n"
f"```python\n"
f"{code}\n"
f"```\n\n"
f"Output:\n"
f"{ex.output}"
) from None
E RuntimeError: Subprocess failed with exit code -6.
E
E Code:
E ```python
E import sys
E sys.path.insert(0, '/wrk/forked/pybind11/tests')
E import standalone_enum_module as m
E assert m.SomeEnum.__class__.__name__ == "pybind11_type"
E ```
E
E Output:
E munmap_chunk(): invalid pointer
_ = 0
code = 'import sys\nsys.path.insert(0, \'/wrk/forked/pybind11/tests\')\nimport standalone_enum_module as m\nassert m.SomeEnum.__class__.__name__ == "pybind11_type"'
os = <module 'os' (frozen)>
rerun = 1
subprocess = <module 'subprocess' from '/wrk/cpython_installs/v3.14.2_df793163d58_default/lib/python3.14/subprocess.py'>
sys = <module 'sys' (built-in)>
textwrap = <module 'textwrap' from '/wrk/cpython_installs/v3.14.2_df793163d58_default/lib/python3.14/textwrap.py'>
env.py:68: RuntimeError
=========================== short test summary info ============================
FAILED test_standalone_enum_module.py::test_enum_import_exit_no_crash - Runti...
============================== 1 failed in 0.23s ===============================
ERROR: completed_process.returncode=1
Collaborator
Author
|
CI results reported here: #6015 (comment) Closing this PR, it served its purpose. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
xref #6015 → xref #5486, #5976, #5991, #6010
This experimental PR is 6015 without the fix in commit 86f3e25
Hoped-for outcome running the CI here: confirms that the new test (commit 67a8a47) reproduces #5976
Suggested changelog entry: