Fix Python reference leaks in message and native helpers - #256
Open
drebbe-intrepid wants to merge 3 commits into
Open
drebbe-intrepid wants to merge 3 commits into
drebbe-intrepid wants to merge 3 commits into
Conversation
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
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.
Repeated message attribute reads and native helper calls retained Python references. Keep attribute names borrowed and use scoped ownership for imported modules, fetched handle attributes, and newly created capsules, including early-return paths.
Add hardware-free regression tests for both message types and native helper success/failure paths. Native tests compile a minimal mock library and run in a subprocess to isolate library overrides and module monkeypatches.
Fixes #243
Validation:
pytest tests -q: 43 passed on each Python version, including a subprocess suite of 14 native-helper tests.git diff --check: passed.The separate reopen/status bug in SetHandle is tracked by #245 and intentionally remains outside this ownership fix. Combined integration requires review of that overlapping setter change.
Review gate: independent critic /root/critic_243 and builder explicitly agree with commit f0a25f9 (reviewed diff SHA256 c7806e4dd4a68f9fc9b977275eff5398bd5d37eabf598c05f83061c1ffc82ad4).
The final review includes five i686 mock export aliases. Both builder and critic independently compiled and inspected the i386 DLL exports; no 32-bit Python runtime was exercised.
Capsule tests compare ownership against a fresh control capsule rather than fixed interpreter-specific counts. An injected extra reference was detected on both CPython 3.12 and 3.14; the critic independently repeated both test runs and sensitivity probes.