Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refcounting changes after PEP 683 #3357

Open
davidhewitt opened this issue Jul 30, 2023 · 3 comments · May be fixed by #4065
Open

Refcounting changes after PEP 683 #3357

davidhewitt opened this issue Jul 30, 2023 · 3 comments · May be fixed by #4065
Labels
Good First Issue
Milestone
0.22

Comments

@davidhewitt
Copy link
Member

PEP 683's "immortal objects" changed the meaning of reference counts; immortal objects have a fixed refcount value which never changes.

Further, the PEP states:

Py_REFCNT() - add “The refcounts 0 and 1 have specific meanings and all others only mean code somewhere is using the object, regardless of the value. 0 means the object is not used and will be cleaned up. 1 means code holds exactly a single reference.”

Does this mean that PyAny::get_refcnt should change, given that the upstream position is essentially that only 0 and 1 have meaning?
@alex
Copy link
Contributor

alex commented Aug 1, 2023

What's the use case for PyAny::get_refcnt? Perhaps it should be deprecated in favor of methods with narrower semantics.

@davidhewitt
Copy link
Member Author

davidhewitt commented Aug 2, 2023 •
edited

I might even suggest it doesn't need a replacement added immediately, we can let users call pyo3::ffi::Py_REFCNT(obj.as_ptr()) to get the same functionality for now and if someone comes up with a desirable proposal for a safe API we could add that then?

@davidhewitt davidhewitt added this to the 0.20 milestone Aug 11, 2023
@davidhewitt davidhewitt modified the milestones: 0.20, 0.21 Oct 2, 2023
@davidhewitt davidhewitt modified the milestones: 0.21, 0.22 Mar 29, 2024
@davidhewitt
Copy link
Member Author

Labelling as "Good First Issue" as all that's really needed is to remove or deprecate .get_refcnt() methods.

lfn3 added a commit to lfn3/pyo3 that referenced this issue Apr 10, 2024
@lfn3
Remove get_refcnt from public api
c9857d4 
This is an attempt to fix PyO3#3357.
The function is used relatively frequently in tests,
so have left it with #[cfg(test)] and pub(crate).
This avoids coating the tests with `unsafe`,
and something that doesn't indicate intent as well.

TODO:
[ ] Fix docs that refer to `get_refcnt` as an example -
    is there another function I can use instead?
lfn3 added a commit to lfn3/pyo3 that referenced this issue Apr 10, 2024
@lfn3
Remove get_refcnt from public api
21ca012 
This is an attempt to fix PyO3#3357.
The function is used relatively frequently in tests,
so have left it with #[cfg(test)] and pub(crate).
This avoids coating the tests with `unsafe`,
and something that doesn't indicate intent as well.

TODO:
[ ] Fix docs that refer to `get_refcnt` as an example -
    is there another function I can use instead?
@lfn3 lfn3 linked a pull request Apr 10, 2024 that will close this issue
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Good First Issue
Projects
None yet
Milestone
0.22
Development

Successfully merging a pull request may close this issue.

Remove get_refcnt from public api lfn3/pyo3
2 participants
@alex @davidhewitt