Skip to content

gh-105927: Add _PyWeakref_GET_REF() internal function #105929

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.

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

Merged
merged 2 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions Include/internal/pycore_weakref.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#ifndef Py_INTERNAL_WEAKREF_H
#define Py_INTERNAL_WEAKREF_H
#ifdef __cplusplus
extern "C" {
#endif

#ifndef Py_BUILD_CORE
# error "this header requires Py_BUILD_CORE define"
#endif

static inline PyObject* _PyWeakref_GET_REF(PyObject *ref_obj) {
assert(PyWeakref_Check(ref_obj));
PyWeakReference *ref = _Py_CAST(PyWeakReference*, ref_obj);
PyObject *obj = ref->wr_object;

if (obj == Py_None) {
// clear_weakref() was called
return NULL;
}

// Explanation for the Py_REFCNT() check: when a weakref's target is part
// of a long chain of deallocations which triggers the trashcan mechanism,
// clearing the weakrefs can be delayed long after the target's refcount
// has dropped to zero. In the meantime, code accessing the weakref will
// be able to "see" the target object even though it is supposed to be
// unreachable. See issue gh-60806.
Py_ssize_t refcnt = Py_REFCNT(obj);
if (refcnt == 0) {
return NULL;
}

assert(refcnt > 0);
return Py_NewRef(obj);
}

#ifdef __cplusplus
}
#endif
#endif /* !Py_INTERNAL_WEAKREF_H */

1 change: 1 addition & 0 deletions Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -1801,6 +1801,7 @@ PYTHON_HEADERS= \
$(srcdir)/Include/internal/pycore_unicodeobject.h \
$(srcdir)/Include/internal/pycore_unicodeobject_generated.h \
$(srcdir)/Include/internal/pycore_warnings.h \
$(srcdir)/Include/internal/pycore_weakref.h \
$(DTRACE_HEADERS) \
@PLATFORM_HEADERS@ \
\
Expand Down
92 changes: 43 additions & 49 deletions Objects/weakrefobject.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "Python.h"
#include "pycore_object.h" // _PyObject_GET_WEAKREFS_LISTPTR()
#include "pycore_weakref.h" // _PyWeakref_GET_REF()
#include "structmember.h" // PyMemberDef


Expand Down Expand Up @@ -147,12 +148,11 @@ weakref_hash(PyWeakReference *self)
{
if (self->hash != -1)
return self->hash;
PyObject* obj = PyWeakref_GET_OBJECT(self);
if (obj == Py_None) {
PyObject* obj = _PyWeakref_GET_REF((PyObject*)self);
if (obj == NULL) {
PyErr_SetString(PyExc_TypeError, "weak object has gone away");
return -1;
}
Py_INCREF(obj);
self->hash = PyObject_Hash(obj);
Py_DECREF(obj);
return self->hash;
Expand All @@ -162,15 +162,13 @@ weakref_hash(PyWeakReference *self)
static PyObject *
weakref_repr(PyObject *self)
{
PyObject *name, *repr;
PyObject* obj = PyWeakref_GET_OBJECT(self);

if (obj == Py_None) {
PyObject* obj = _PyWeakref_GET_REF(self);
if (obj == NULL) {
return PyUnicode_FromFormat("<weakref at %p; dead>", self);
}

Py_INCREF(obj);
name = _PyObject_LookupSpecial(obj, &_Py_ID(__name__));
PyObject *name = _PyObject_LookupSpecial(obj, &_Py_ID(__name__));
PyObject *repr;
if (name == NULL || !PyUnicode_Check(name)) {
repr = PyUnicode_FromFormat(
"<weakref at %p; to '%s' at %p>",
Expand All @@ -191,16 +189,18 @@ weakref_repr(PyObject *self)
gone away, they are equal if they are identical. */

static PyObject *
weakref_richcompare(PyWeakReference* self, PyWeakReference* other, int op)
weakref_richcompare(PyObject* self, PyObject* other, int op)
{
if ((op != Py_EQ && op != Py_NE) ||
!PyWeakref_Check(self) ||
!PyWeakref_Check(other)) {
Py_RETURN_NOTIMPLEMENTED;
}
PyObject* obj = PyWeakref_GET_OBJECT(self);
PyObject* other_obj = PyWeakref_GET_OBJECT(other);
if (obj == Py_None || other_obj == Py_None) {
PyObject* obj = _PyWeakref_GET_REF(self);
PyObject* other_obj = _PyWeakref_GET_REF(other);
if (obj == NULL || other_obj == NULL) {
Py_XDECREF(obj);
Py_XDECREF(other_obj);
int res = (self == other);
if (op == Py_NE)
res = !res;
Expand All @@ -209,8 +209,6 @@ weakref_richcompare(PyWeakReference* self, PyWeakReference* other, int op)
else
Py_RETURN_FALSE;
}
Py_INCREF(obj);
Py_INCREF(other_obj);
PyObject* res = PyObject_RichCompare(obj, other_obj, op);
Py_DECREF(obj);
Py_DECREF(other_obj);
Expand Down Expand Up @@ -372,7 +370,7 @@ _PyWeakref_RefType = {
Py_TPFLAGS_HAVE_VECTORCALL | Py_TPFLAGS_BASETYPE,
.tp_traverse = (traverseproc)gc_traverse,
.tp_clear = (inquiry)gc_clear,
.tp_richcompare = (richcmpfunc)weakref_richcompare,
.tp_richcompare = weakref_richcompare,
.tp_methods = weakref_methods,
.tp_members = weakref_members,
.tp_init = weakref___init__,
Expand All @@ -385,7 +383,7 @@ _PyWeakref_RefType = {
static bool
proxy_check_ref(PyObject *obj)
{
if (obj == Py_None) {
if (obj == NULL) {
PyErr_SetString(PyExc_ReferenceError,
"weakly-referenced object no longer exists");
return false;
Expand All @@ -400,16 +398,19 @@ proxy_check_ref(PyObject *obj)
*/
#define UNWRAP(o) \
if (PyWeakref_CheckProxy(o)) { \
o = PyWeakref_GET_OBJECT(o); \
if (!proxy_check_ref(o)) \
o = _PyWeakref_GET_REF(o); \
if (!proxy_check_ref(o)) { \
return NULL; \
} \
} \
else { \
Py_INCREF(o); \
}

#define WRAP_UNARY(method, generic) \
static PyObject * \
method(PyObject *proxy) { \
UNWRAP(proxy); \
Py_INCREF(proxy); \
PyObject* res = generic(proxy); \
Py_DECREF(proxy); \
return res; \
Expand All @@ -420,8 +421,6 @@ proxy_check_ref(PyObject *obj)
method(PyObject *x, PyObject *y) { \
UNWRAP(x); \
UNWRAP(y); \
Py_INCREF(x); \
Py_INCREF(y); \
PyObject* res = generic(x, y); \
Py_DECREF(x); \
Py_DECREF(y); \
Expand All @@ -436,11 +435,9 @@ proxy_check_ref(PyObject *obj)
method(PyObject *proxy, PyObject *v, PyObject *w) { \
UNWRAP(proxy); \
UNWRAP(v); \
if (w != NULL) \
if (w != NULL) { \
UNWRAP(w); \
Py_INCREF(proxy); \
Py_INCREF(v); \
Py_XINCREF(w); \
} \
PyObject* res = generic(proxy, v, w); \
Py_DECREF(proxy); \
Py_DECREF(v); \
Expand All @@ -452,7 +449,6 @@ proxy_check_ref(PyObject *obj)
static PyObject * \
method(PyObject *proxy, PyObject *Py_UNUSED(ignored)) { \
UNWRAP(proxy); \
Py_INCREF(proxy); \
PyObject* res = PyObject_CallMethodNoArgs(proxy, &_Py_ID(SPECIAL)); \
Py_DECREF(proxy); \
return res; \
Expand All @@ -466,24 +462,24 @@ WRAP_UNARY(proxy_str, PyObject_Str)
WRAP_TERNARY(proxy_call, PyObject_Call)

static PyObject *
proxy_repr(PyWeakReference *proxy)
proxy_repr(PyObject *proxy)
{
return PyUnicode_FromFormat(
PyObject *obj = _PyWeakref_GET_REF(proxy);
PyObject *repr = PyUnicode_FromFormat(
"<weakproxy at %p to %s at %p>",
proxy,
Py_TYPE(PyWeakref_GET_OBJECT(proxy))->tp_name,
PyWeakref_GET_OBJECT(proxy));
proxy, Py_TYPE(obj)->tp_name, obj);
Py_DECREF(obj);
return repr;
}


static int
proxy_setattr(PyObject *proxy, PyObject *name, PyObject *value)
{
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
PyObject *obj = _PyWeakref_GET_REF(proxy);
if (!proxy_check_ref(obj)) {
return -1;
}
Py_INCREF(obj);
int res = PyObject_SetAttr(obj, name, value);
Py_DECREF(obj);
return res;
Expand All @@ -494,7 +490,10 @@ proxy_richcompare(PyObject *proxy, PyObject *v, int op)
{
UNWRAP(proxy);
UNWRAP(v);
return PyObject_RichCompare(proxy, v, op);
PyObject* res = PyObject_RichCompare(proxy, v, op);
Py_DECREF(proxy);
Py_DECREF(v);
return res;
}

/* number slots */
Expand Down Expand Up @@ -536,11 +535,10 @@ WRAP_BINARY(proxy_imatmul, PyNumber_InPlaceMatrixMultiply)
static int
proxy_bool(PyObject *proxy)
{
PyObject *o = PyWeakref_GET_OBJECT(proxy);
PyObject *o = _PyWeakref_GET_REF(proxy);
if (!proxy_check_ref(o)) {
return -1;
}
Py_INCREF(o);
int res = PyObject_IsTrue(o);
Py_DECREF(o);
return res;
Expand All @@ -561,11 +559,10 @@ proxy_dealloc(PyWeakReference *self)
static int
proxy_contains(PyObject *proxy, PyObject *value)
{
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
PyObject *obj = _PyWeakref_GET_REF(proxy);
if (!proxy_check_ref(obj)) {
return -1;
}
Py_INCREF(obj);
int res = PySequence_Contains(obj, value);
Py_DECREF(obj);
return res;
Expand All @@ -576,11 +573,10 @@ proxy_contains(PyObject *proxy, PyObject *value)
static Py_ssize_t
proxy_length(PyObject *proxy)
{
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
PyObject *obj = _PyWeakref_GET_REF(proxy);
if (!proxy_check_ref(obj)) {
return -1;
}
Py_INCREF(obj);
Py_ssize_t res = PyObject_Length(obj);
Py_DECREF(obj);
return res;
Expand All @@ -591,11 +587,10 @@ WRAP_BINARY(proxy_getitem, PyObject_GetItem)
static int
proxy_setitem(PyObject *proxy, PyObject *key, PyObject *value)
{
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
PyObject *obj = _PyWeakref_GET_REF(proxy);
if (!proxy_check_ref(obj)) {
return -1;
}
Py_INCREF(obj);
int res;
if (value == NULL) {
res = PyObject_DelItem(obj, key);
Expand All @@ -611,11 +606,10 @@ proxy_setitem(PyObject *proxy, PyObject *key, PyObject *value)
static PyObject *
proxy_iter(PyObject *proxy)
{
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
PyObject *obj = _PyWeakref_GET_REF(proxy);
if (!proxy_check_ref(obj)) {
return NULL;
}
Py_INCREF(obj);
PyObject* res = PyObject_GetIter(obj);
Py_DECREF(obj);
return res;
Expand All @@ -624,17 +618,17 @@ proxy_iter(PyObject *proxy)
static PyObject *
proxy_iternext(PyObject *proxy)
{
PyObject *obj = PyWeakref_GET_OBJECT(proxy);
PyObject *obj = _PyWeakref_GET_REF(proxy);
if (!proxy_check_ref(obj)) {
return NULL;
}
if (!PyIter_Check(obj)) {
PyErr_Format(PyExc_TypeError,
"Weakref proxy referenced a non-iterator '%.200s' object",
Py_TYPE(obj)->tp_name);
Py_DECREF(obj);
return NULL;
}
Py_INCREF(obj);
PyObject* res = PyIter_Next(obj);
Py_DECREF(obj);
return res;
Expand Down Expand Up @@ -721,7 +715,7 @@ _PyWeakref_ProxyType = {
0, /* tp_getattr */
0, /* tp_setattr */
0, /* tp_as_async */
(reprfunc)proxy_repr, /* tp_repr */
proxy_repr, /* tp_repr */
&proxy_as_number, /* tp_as_number */
&proxy_as_sequence, /* tp_as_sequence */
&proxy_as_mapping, /* tp_as_mapping */
Expand Down Expand Up @@ -756,7 +750,7 @@ _PyWeakref_CallableProxyType = {
0, /* tp_getattr */
0, /* tp_setattr */
0, /* tp_as_async */
(unaryfunc)proxy_repr, /* tp_repr */
proxy_repr, /* tp_repr */
&proxy_as_number, /* tp_as_number */
&proxy_as_sequence, /* tp_as_sequence */
&proxy_as_mapping, /* tp_as_mapping */
Expand Down
1 change: 1 addition & 0 deletions PCbuild/pythoncore.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@
<ClInclude Include="..\Include\internal\pycore_unicodeobject.h" />
<ClInclude Include="..\Include\internal\pycore_unicodeobject_generated.h" />
<ClInclude Include="..\Include\internal\pycore_warnings.h" />
<ClInclude Include="..\Include\internal\pycore_weakref.h" />
<ClInclude Include="..\Include\interpreteridobject.h" />
<ClInclude Include="..\Include\intrcheck.h" />
<ClInclude Include="..\Include\iterobject.h" />
Expand Down
3 changes: 3 additions & 0 deletions PCbuild/pythoncore.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,9 @@
<ClInclude Include="..\Include\internal\pycore_warnings.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_weakref.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_abstract.h">
<Filter>Include\internal</Filter>
</ClInclude>
Expand Down