Skip to content

Conversation

@efimov-mikhail
Copy link
Member

@efimov-mikhailefimov-mikhail commented Oct 25, 2025

@efimov-mikhail
Copy link
MemberAuthor

It works fine and generally follows the sketch in the issue.
But I don't follow chains of borrows and check only the nearest borrowed_from stackref.

There are 17 failed tests with this PR in Py_STACKREF_DEBUG mode.

17 tests failed: test.test_gdb.test_backtrace test.test_gdb.test_misc test.test_gdb.test_pretty_print test.test_io.test_general test.test_multiprocessing_fork.test_misc test.test_multiprocessing_forkserver.test_misc test.test_multiprocessing_spawn.test_manager test.test_multiprocessing_spawn.test_misc test_external_inspection test_faulthandler test_frame test_interpreters test_profiling test_pyrepl test_regrtest test_repl test_threading 

Compare with
#139475 (comment)

Here test.test_io.test_general is a bit flaky, but test_frame is a real find by new borrow checker.

-> % ./python -m unittest -v test.test_frame.TestFrameLocals.test_overwrite_locals test_overwrite_locals (test.test_frame.TestFrameLocals.test_overwrite_locals) ... Fatal Python error: _Py_stackref_close: StackRef with ID 2401080 closed while borrows 1 refs at Objects/frameobject.c:288. Opened at Python/generated_cases.c.h:1239 

There's no real problem here but we can rewrite it a little to prevent this error and make code related to frame->f_overwritten_fast_locals clearer.

Copy link
Contributor

@mpagempage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Just some minor inline comments/suggestions. Feel free to push back if you don't agree.

Copy link
Contributor

@mpagempage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks!

@mpagempage merged commit 3cb1ab0 into python:mainNov 5, 2025
83 of 85 checks passed
@efimov-mikhail
Copy link
MemberAuthor

efimov-mikhail commented Nov 5, 2025

Thanks a lot for merge, @mpage!

Maybe you'll be interesting in another stackref related PR?
#139717

StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
Add borrow checking to the stackref debug mode --------- Co-authored-by: mpage <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@efimov-mikhail@mpage