clear_weakref() in Objects/weakrefobject.c is Confusing
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 77.2k
- Forks
- 36k
- PR merge metrics
- PR metrics pending
Description
(inspired by https://github.com/python/cpython/pull/95302#discussion_r931261376)
In Objects/weakrefobject.c, the clear_weakref() function is used to:
- pop the
PyWeakReferenceobject off its referenced object'stp_weaklist(a linked list) - clear (but not call) the callback, if any
However, in the places it is used, it isn't clear what it's doing. When adding a new use of clear_weakref() in one of my PRs it was particularly confusing to a reviewer what the relationship was between clear_weakref() and an object's weakref list (i.e. tp_weaklist).
While we could simply rename it to "remove_and_clear_weakref", it may make sense to do the following:
- move the linked-list part to a separate
remove_weakref()that has an explicitPyWeakReference **parameter - (hence, remove the linked list parts from
clear_weakref()) - add
remove_weakref_from_referent()which callsGET_WEAKREFS_LISTPTR(self->wr_object)and callsremove_weakref()on it - add a call to it everywhere we already call
clear_weakref()
Other observations:
- currently we aren't reseting
self->hashorself->vectorcallto NULL inclear_weakref() - we may be leaking weakref objects
PyObject_ClearWeakRefs()when the weakref doesn't have a callback
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in Objects/weakrefobject.c by reading clear_weakref(), its callers, and the handling of tp_weaklist. Compare the proposed remove_weakref() and remove_weakref_from_referent() separation with the linked PR discussion, then investigate the hash, vectorcall, and possible weakref-leak observations. Done means the confusing responsibilities and remaining concerns have a decided, reviewed scope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, python
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100