Skip to content

gh-156831: Fix reference ownership in _testcapi.sequence_fast_get_item - #156832

Closed
BHUVANSH855 wants to merge 1 commit into
python:mainfrom
BHUVANSH855:fix-156831-sequence-fast-ref-pr
Closed

gh-156831: Fix reference ownership in _testcapi.sequence_fast_get_item#156832
BHUVANSH855 wants to merge 1 commit into
python:mainfrom
BHUVANSH855:fix-156831-sequence-fast-ref-pr

Conversation

@BHUVANSH855

Copy link
Copy Markdown
Contributor

gh-156831

_testcapi.sequence_fast_get_item() returns the value from
PySequence_Fast_GET_ITEM() directly.

PySequence_Fast_GET_ITEM() returns a borrowed reference, so the wrapper
needs to create a new reference before returning the object.

Use Py_NewRef() for the returned item and add a regression test that checks
the reference count is increased by one.

Tests:

  • ./python -m test test_capi.test_abstract -v

@BHUVANSH855 BHUVANSH855 changed the title Fix reference ownership in _testcapi.sequence_fast_get_item gh-156831: Fix reference ownership in _testcapi.sequence_fast_get_item Sep 2, 2026

@StanFromIreland StanFromIreland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is an internal function, are our tests impacted by this? I agree with Bénédikt, I'm not sure why this needs to change.

@StanFromIreland StanFromIreland added the pending The issue will be closed if no feedback is provided label Sep 2, 2026
@ZeroIntensity

Copy link
Copy Markdown
Member

It's not an issue in our tests because the items returned by sequence_fast_get_item are all immortal. Let's hold off on this until it actually causes an issue in practice; for now, this is just churn that would encourage more changes like this.

@ZeroIntensity ZeroIntensity removed the pending The issue will be closed if no feedback is provided label Sep 2, 2026
@BHUVANSH855
BHUVANSH855 deleted the fix-156831-sequence-fast-ref-pr branch September 2, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants