python / python/cpython

clear_weakref() in Objects/weakrefobject.c is Confusing

Abierto
#95,348 2 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

interpreter-core type-feature
Lenguaje dominante
Python
Estrellas
77.2k
Forks
36k
Métricas de merge de PR
Métricas de PR pendientes

Descripción

(inspired by https://github.com/python/cpython/pull/95302#discussion_r931261376)

In Objects/weakrefobject.c, the clear_weakref() function is used to:

  • pop the PyWeakReference object off its referenced object's tp_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 explicit PyWeakReference ** parameter
  • (hence, remove the linked list parts from clear_weakref())
  • add remove_weakref_from_referent() which calls GET_WEAKREFS_LISTPTR(self->wr_object) and calls remove_weakref() on it
  • add a call to it everywhere we already call clear_weakref()

Other observations:

  • currently we aren't reseting self->hash or self->vectorcall to NULL in clear_weakref()
  • we may be leaking weakref objects PyObject_ClearWeakRefs() when the weakref doesn't have a callback

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Empieza en Objects/weakrefobject.c leyendo clear_weakref(), sus llamadores y el tratamiento de tp_weaklist. Compara la separación propuesta entre remove_weakref() y remove_weakref_from_referent() con la discusión del PR enlazado y, después, investiga las observaciones sobre hash, vectorcall y posibles weakref-leak. Se considera terminado cuando se haya decidido y revisado el alcance de las responsabilidades confusas y las preocupaciones restantes.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
c, python
Área
backend
Tipo de issue
Refactorización
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Estancado
Claridad
Bastante claro
Aptitud para principiantes
35/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.