Skip to content

Track PickleBuffer reference cycles - #9733

Merged
youknowone merged 2 commits into
RustPython:mainfrom
youknowdot:fix/picklebuffer-cycle-batch-20261107
Nov 7, 2026
Merged

Track PickleBuffer reference cycles#9733
youknowone merged 2 commits into
RustPython:mainfrom
youknowdot:fix/picklebuffer-cycle-batch-20261107

Conversation

@youknowdot

@youknowdot youknowdot commented Nov 6, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Stack and CI

Stacked on #9732 (f6d22b4e0a). The two PickleBuffer commits are the new batch; the preceding LZMA changes belong to that parent PR.

The final commit carries [skip ci] to avoid launching another heavy hosted run while the parent is pending. Adding the skip:ci label was denied by repository permissions, so that label is not applied. The commit directive suppresses the ordinary CI workflows; the lightweight Lib Dependencies check can still run. Hosted platform CI for this head is intentionally deferred, not claimed green. Rebase after #9732 lands and remove the skip directive when the stack is ready for its platform checks.

Summary

  • Enable the existing traversal for PyPickleBuffer so its owned exporter reference is visible to cyclic GC.
  • Reuse PyBuffer and PyRwLock traversal without cloning references or running exporter callbacks. Preserve acquisition and release ordering.
  • Remove the sole remaining expected-failure marker in test_picklebuffer.py using scripts/update_lib quick Lib/test/test_picklebuffer.py --no-build --no-commit.

Relative to #9732, exactly two files change: one Rust attribute and one removed decorator. No assertions, test logic, skips, duplicate snippets, or workflows change.

Validation

  • Fresh release interpreter with an explicit worktree RUSTPYTHONPATH; imported canonical test location verified.
  • Complete test_picklebuffer: 9 tests, 3 unchanged _testbuffer availability skips, zero expected failures.
  • Complete test_memoryview, test_gc, and test_pickle suites passed, retaining their existing skips/expected failures.
  • test_buffer.TestPythonBufferProtocol passed: 19 tests, 4 existing skips.
  • Targeted GC inspection matches CPython 3.15: tracked buffer, one exporter edge, cycle collected; release removes the edge.
  • Full workspace Rust tests passed, excluding the separately configured C-API, WASM, and Windows venv launcher crates.
  • Separate C-API tests passed: 149 passed, 2 existing ignored tests.
  • Workspace and separate C-API all-target Clippy passed with -D warnings.
  • Independent traversal/refcount/lock/callback review and normal commit hooks passed.

No native non-Linux or hosted validation is claimed. Open #8939 still lacks this traversal flag and does not duplicate the correction.

AI assistance: OpenAI Codex performed inspection, implementation, independent review, and validation. The exact model version was not exposed.

Summary by CodeRabbit

  • No user-facing changes — This update does not change user-visible behavior.

@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Explain Change →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Files ignored due to path filters: 1

  • Lib/test/test_picklebuffer.py is excluded by !Lib/**
Run configuration

Commits

Reviewing files that changed from the base of the PR and between e4fdaa9 and 48baa9d.

Files ignored due to path filters: 1

  • Lib/test/test_picklebuffer.py is excluded by !Lib/**

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The PyPickleBuffer class declaration adds the traverse option. Its fields and methods remain unchanged.

Changes: PyPickleBuffer traversal

Layer / File(s) Summary
Add traversal support
crates/stdlib/src/pickle.rs		 The PyPickleBuffer declaration adds the traverse option. Other declaration attributes remain unchanged.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Possibly related PRs

Merge Risk: Minimal · previous review e4fda

The change exposes PickleBuffer reference cycles to garbage collection while preserving buffer acquisition and release behavior. No actionable merge-blocking risk remains after normal checks.

Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main change: enabling reference-cycle tracking for PickleBuffer.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between fb57e68 and e4fdaa9.

Files ignored due to path filters: 1

  • Lib/test/test_picklebuffer.py is excluded by !Lib/**
Files selected for processing: 1

  • crates/stdlib/src/pickle.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

@github-actions

github-actions Bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/pickle.py
[ ] lib: cpython/Lib/_compat_pickle.py
[x] test: cpython/Lib/test/picklecommon.py
[ ] test: cpython/Lib/test/test_pickle.py (TODO: 3)
[x] test: cpython/Lib/test/test_picklebuffer.py
[x] test: cpython/Lib/test/test_pickletools.py
[x] test: cpython/Lib/test/test_xpickle.py
[x] test: cpython/Lib/test/xpickle_worker.py

dependencies:

  • pickle (native: _pickle, itertools, sys)
    • _compat_pickle
    • _compat_pickle
    • io (native: _io, _thread, errno, msvcrt, sys)
    • argparse, codecs, copyreg, functools, pprint, struct, types

dependent tests: (102 tests)

  • pickle: test_annotationlib test_argparse test_array test_ast test_asyncio test_bool test_builtin test_bytes test_bz2 test_codecs test_collections test_concurrent_futures test_configparser test_coroutines test_csv test_ctypes test_decimal test_defaultdict test_deque test_descr test_dict test_dictviews test_email test_enum test_enumerate test_exceptions test_external_inspection test_fractions test_free_threading test_functools test_generators test_genericalias test_genericpath test_http_cookies test_importlib test_inspect test_interpreters test_io test_ipaddress test_iter test_itertools test_json test_list test_logging test_lzma test_memoryview test_minidom test_opcache test_operator test_ordered_dict test_os test_pathlib test_pickle test_picklebuffer test_pickletools test_platform test_plistlib test_positional_only_arg test_random test_range test_re test_set test_shelve test_slice test_socket test_statistics test_str test_string test_structseq test_super test_time test_trace test_tuple test_turtle test_type_aliases test_type_params test_types test_typing test_unittest test_urlparse test_uuid test_xml_dom_minicompat test_xml_etree test_xpickle test_zipfile test_zlib test_zoneinfo
    • logging: test_asyncio test_concurrent_futures test_hashlib test_pkgutil test_support test_urllib2net
      • asyncio.futures: test_asyncio
      • concurrent.futures._base: test_concurrent_futures
      • http.cookiejar: test_http_cookiejar test_urllib2
      • multiprocessing.util: test_asyncio test_compileall test_concurrent_futures
      • venv: test_venv
    • tracemalloc: test_tracemalloc

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@youknowdot youknowdot mentioned this pull request Nov 6, 2026
Merged
@codex
Track PickleBuffer exporter references during cyclic GC
5d6fc12 
Use the existing PyBuffer and PyRwLock traversal so exporter-to-PickleBuffer cycles are visible to the collector. Preserve buffer acquisition, release, and callback ordering.

Assisted-by: OpenAI Codex: model version not exposed
@youknowdot
youknowdot force-pushed the fix/picklebuffer-cycle-batch-20261107 branch from 2890cbb to e4fdaa9 Compare November 6, 2026 23:48
@youknowone
youknowone marked this pull request as ready for review November 7, 2026 00:14
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 7, 2026

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-07T00:14:52.921678Z
We couldn’t complete this review. Comment @codex review to try again.		 e4fdaa9 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@youknowone
youknowone enabled auto-merge (squash) November 7, 2026 00:14
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 7, 2026

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@codex
Retire the remaining PickleBuffer expected failure
48baa9d 
Use scripts/update_lib quick Lib/test/test_picklebuffer.py --no-build --no-commit after enabling exporter traversal. All nine canonical tests pass with the three existing _testbuffer availability skips; no assertions, test logic, or skips change.

Assisted-by: OpenAI Codex: model version not exposed
auto-merge was automatically disabled November 7, 2026 06:18

Head branch was pushed to by a user without write access

@youknowdot
youknowdot force-pushed the fix/picklebuffer-cycle-batch-20261107 branch from e4fdaa9 to 48baa9d Compare November 7, 2026 06:18
@youknowone
youknowone merged commit cf17ec3 into RustPython:main Nov 7, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@youknowdot @youknowone @codex