gh-135125: Fix Py_STACKREF_DEBUG build

[efimov-mikhail]

There are two problems with PyStackRef debug builds:

1. Incorrect _PyStackRef_FromPyObjectBorrow function.
   We should provide Py_INCREF here, because each PyStackRef_CLOSE will use Py_DECREF in this mode.
2. No PyStackRef_Wrap and PyStackRef_Unwrap functions.

• Issue: Build fails with Py_STACKREF_DEBUG set. #135125

[efimov-mikhail]

CC @markshannon

[efimov-mikhail]

This PR is just minimal fix for Py_STACKREF_DEBUG build.

I suggest to make changes for this build: provide exactly those flags for indexes as for standard GIL build:

#define Py_INT_TAG 3 #define Py_TAG_INVALID 2 #define Py_TAG_REFCNT 1 #define Py_TAG_BITS 3 

And use indexes without flags as real indexes in hash table.
And make all refcounts for DUP, CLOSE and Borrow exactly the same as for standard GIL build.

For example, test.test_list.ListTest.test_list_overwrite_local fails with current variant of PyStackRef debug build because refcounts differ.

[efimov-mikhail]

This PR is just minimal fix for Py_STACKREF_DEBUG build.

I suggest to make changes for this build: provide exactly those flags for indexes as for standard GIL build:

#define Py_INT_TAG 3 #define Py_TAG_INVALID 2 #define Py_TAG_REFCNT 1 #define Py_TAG_BITS 3 

And use indexes without flags as real indexes in hash table. And make all refcounts for DUP, CLOSE and Borrow exactly the same as for standard GIL build.

For example, test.test_list.ListTest.test_list_overwrite_local fails with current variant of PyStackRef debug build because refcounts differ.

I provide those changes.
IMO, this variant is better than minimal fix:

1. Two tests test_list and test_capi don't fail with this but fail with minimal fix.
2. We have exactly the same refcount scheme and it's easier to understand what's going on.
3. Future changes for StackRef build will be more obvious.

[efimov-mikhail]

CC @mpage

[mpage]

This looks good to me. In the future I'd recommend splitting this into two PRs (and associated issues): one that is the minimal fix for the build and another that unifies the refcounting behavior. I'd like to give Mark a chance to review this as I'm not sure if he wants to preserve the refcounting behavior in the debug build. I've added him as a reviewer.

[efimov-mikhail]

This looks good to me. In the future I'd recommend splitting this into two PRs (and associated issues): one that is the minimal fix for the build and another that unifies the refcounting behavior. I'd like to give Mark a chance to review this as I'm not sure if he wants to preserve the refcounting behavior in the debug build. I've added him as a reviewer.

Thanks! I thought about two separate PRs but I wasn't sure about that.
I can make another PR with minimal fix if needed.

[sergey-miryanov]

Since #139384 has been merged, we should add PyStackRef_IsMalformed. Do you plan to add it in this PR or in a follow-up one?

[efimov-mikhail]

Since #139384 has been merged, we should add PyStackRef_IsMalformed. Do you plan to add it in this PR or in a follow-up one?

I've already added this function to Include/internal/pycore_stackref.h.

[sergey-miryanov]

I've already added this function to Include/internal/pycore_stackref.h.

Sorry, it is my oversight.

[sergey-miryanov]

LGTM. Thanks!

[efimov-mikhail]

@markshannon
Could you please take a look at this?

[markshannon]

This looks good, and passes all the test on my machine with #define Py_STACKREF_DEBUG 1 but I'm puzzled by one thing.

When I last run all the tests with #define Py_STACKREF_DEBUG 1, many moths ago before I broke them, a number of tests failed due to leaks when stopping threads.
Running today, all the tests pass.

@efimov-mikhail any idea why all the tests now pass? Have the leaks been fixed, or are we not now detecting them for some reason?

[efimov-mikhail]

Thanks for the review!
It's kinda strange that all tests are passed in Py_STACKREF_DEBUG mode.
Perhaps something was compiled or run incorrectly.

On my machine I have the following failures in this mode:

15 tests failed: test.test_gdb.test_backtrace test.test_gdb.test_misc test.test_gdb.test_pretty_print 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_interpreters test_profiling test_pyrepl test_regrtest test_repl test_threading 

[markshannon]

Those failures look very much like the ones I was seeing before.
I must have done something wrong, let me try again.

[markshannon]

Yeah, I had #define Py_STACKREF_CLOSE_DEBUG 1 by mistake, which has no effect without #define Py_STACKREF_DEBUG 1.

All looks good now.

[markshannon]

Thanks for doing this, btw.

[efimov-mikhail]

Thanks for merge! Do we want to backport this on 3.14 or it seems redundant?