Issue #28427: old keys should not remove new values from · pythoncapi/cpython@e10ca3a · GitHub
Skip to content

Commit e10ca3a

Browse files
pitroupitrou
committed
Issue python#28427: old keys should not remove new values from
WeakValueDictionary when collecting from another thread.
1 parent 1fee515 commit e10ca3a

7 files changed

Lines changed: 174 additions & 30 deletions

File tree

‎Include/dictobject.h‎

Lines changed: 2 additions & 0 deletions

‎Lib/test/test_weakref.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1673,6 +1673,18 @@ def test_threaded_weak_valued_pop(self):
16731673
x = d.pop(10, 10)
16741674
self.assertIsNot(x, None) # we never put None in there!
16751675

1676+
def test_threaded_weak_valued_consistency(self):
1677+
# Issue #28427: old keys should not remove new values from
1678+
# WeakValueDictionary when collecting from another thread.
1679+
d = weakref.WeakValueDictionary()
1680+
with collect_in_thread():
1681+
for i in range(200000):
1682+
o = RefCycle()
1683+
d[10] = o
1684+
# o is still alive, so the dict can't be empty
1685+
self.assertEqual(len(d), 1)
1686+
o = None # lose ref
1687+
16761688

16771689
from test import mapping_tests
16781690

‎Lib/weakref.py‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
proxy,
1717
CallableProxyType,
1818
ProxyType,
19-
ReferenceType)
19+
ReferenceType,
20+
_remove_dead_weakref)
2021

2122
from _weakrefset import WeakSet, _IterationGuard
2223

@@ -111,7 +112,9 @@ def remove(wr, selfref=ref(self)):
111112
if self._iterating:
112113
self._pending_removals.append(wr.key)
113114
else:
114-
del self.data[wr.key]
115+
# Atomic removal is necessary since this function
116+
# can be called asynchronously by the GC
117+
_remove_dead_weakref(d, wr.key)
115118
self._remove = remove
116119
# A list of keys to be removed
117120
self._pending_removals = []
@@ -125,9 +128,12 @@ def _commit_removals(self):
125128
# We shouldn't encounter any KeyError, because this method should
126129
# always be called *before* mutating the dict.
127130
while l:
128-
del d[l.pop()]
131+
key = l.pop()
132+
_remove_dead_weakref(d, key)
129133

130134
def __getitem__(self, key):
135+
if self._pending_removals:
136+
self._commit_removals()
131137
o = self.data[key]()
132138
if o is None:
133139
raise KeyError(key)
@@ -140,9 +146,13 @@ def __delitem__(self, key):
140146
del self.data[key]
141147

142148
def __len__(self):
143-
return len(self.data) - len(self._pending_removals)
149+
if self._pending_removals:
150+
self._commit_removals()
151+
return len(self.data)
144152

145153
def __contains__(self, key):
154+
if self._pending_removals:
155+
self._commit_removals()
146156
try:
147157
o = self.data[key]()
148158
except KeyError:
@@ -158,6 +168,8 @@ def __setitem__(self, key, value):
158168
self.data[key] = KeyedRef(value, self._remove, key)
159169

160170
def copy(self):
171+
if self._pending_removals:
172+
self._commit_removals()
161173
new = WeakValueDictionary()
162174
for key, wr in self.data.items():
163175
o = wr()
@@ -169,6 +181,8 @@ def copy(self):
169181

170182
def __deepcopy__(self, memo):
171183
from copy import deepcopy
184+
if self._pending_removals:
185+
self._commit_removals()
172186
new = self.__class__()
173187
for key, wr in self.data.items():
174188
o = wr()
@@ -177,6 +191,8 @@ def __deepcopy__(self, memo):
177191
return new
178192

179193
def get(self, key, default=None):
194+
if self._pending_removals:
195+
self._commit_removals()
180196
try:
181197
wr = self.data[key]
182198
except KeyError:
@@ -190,13 +206,17 @@ def get(self, key, default=None):
190206
return o
191207

192208
def items(self):
209+
if self._pending_removals:
210+
self._commit_removals()
193211
with _IterationGuard(self):
194212
for k, wr in self.data.items():
195213
v = wr()
196214
if v is not None:
197215
yield k, v
198216

199217
def keys(self):
218+
if self._pending_removals:
219+
self._commit_removals()
200220
with _IterationGuard(self):
201221
for k, wr in self.data.items():
202222
if wr() is not None:
@@ -214,10 +234,14 @@ def itervaluerefs(self):
214234
keep the values around longer than needed.
215235
216236
"""
237+
if self._pending_removals:
238+
self._commit_removals()
217239
with _IterationGuard(self):
218240
yield from self.data.values()
219241

220242
def values(self):
243+
if self._pending_removals:
244+
self._commit_removals()
221245
with _IterationGuard(self):
222246
for wr in self.data.values():
223247
obj = wr()
@@ -290,6 +314,8 @@ def valuerefs(self):
290314
keep the values around longer than needed.
291315
292316
"""
317+
if self._pending_removals:
318+
self._commit_removals()
293319
return list(self.data.values())
294320

295321

‎Misc/NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ Core and Builtins
138138
Library
139139
-------
140140

141+
- Issue #28427: old keys should not remove new values from
142+
WeakValueDictionary when collecting from another thread.
143+
141144
- Issue 28923: Remove editor artifacts from Tix.py.
142145

143146
- Issue #28871: Fixed a crash when deallocate deep ElementTree.

‎Modules/_weakref.c‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,46 @@ _weakref_getweakrefcount_impl(PyObject *module, PyObject *object)
3535
}
3636

3737

38+
static int
39+
is_dead_weakref(PyObject *value)
40+
{
41+
if (!PyWeakref_Check(value)) {
42+
PyErr_SetString(PyExc_TypeError, "not a weakref");
43+
return -1;
44+
}
45+
return PyWeakref_GET_OBJECT(value) == Py_None;
46+
}
47+
48+
/*[clinic input]
49+
50+
_weakref._remove_dead_weakref -> object
51+
52+
dct: object(subclass_of='&PyDict_Type')
53+
key: object
54+
/
55+
56+
Atomically remove key from dict if it points to a dead weakref.
57+
[clinic start generated code]*/
58+
59+
static PyObject *
60+
_weakref__remove_dead_weakref_impl(PyObject *module, PyObject *dct,
61+
PyObject *key)
62+
/*[clinic end generated code: output=d9ff53061fcb875c input=19fc91f257f96a1d]*/
63+
{
64+
if (_PyDict_DelItemIf(dct, key, is_dead_weakref) < 0) {
65+
if (PyErr_ExceptionMatches(PyExc_KeyError))
66+
/* This function is meant to allow safe weak-value dicts
67+
with GC in another thread (see issue #28427), so it's
68+
ok if the key doesn't exist anymore.
69+
*/
70+
PyErr_Clear();
71+
else
72+
return NULL;
73+
}
74+
Py_RETURN_NONE;
75+
}
76+
77+
3878
PyDoc_STRVAR(weakref_getweakrefs__doc__,
3979
"getweakrefs(object) -- return a list of all weak reference objects\n"
4080
"that point to 'object'.");
@@ -88,6 +128,7 @@ weakref_proxy(PyObject *self, PyObject *args)
88128
static PyMethodDef
89129
weakref_functions[] = {
90130
_WEAKREF_GETWEAKREFCOUNT_METHODDEF
131+
_WEAKREF__REMOVE_DEAD_WEAKREF_METHODDEF
91132
{"getweakrefs", weakref_getweakrefs, METH_O,
92133
weakref_getweakrefs__doc__},
93134
{"proxy", weakref_proxy, METH_VARARGS,

‎Modules/clinic/_weakref.c.h‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,33 @@ _weakref_getweakrefcount(PyObject *module, PyObject *object)
2828
exit:
2929
return return_value;
3030
}
31-
/*[clinic end generated code: output=d9086c8576d46933 input=a9049054013a1b77]*/
31+
32+
PyDoc_STRVAR(_weakref__remove_dead_weakref__doc__,
33+
"_remove_dead_weakref($module, dct, key, /)\n"
34+
"--\n"
35+
"\n"
36+
"Atomically remove key from dict if it points to a dead weakref.");
37+
38+
#define _WEAKREF__REMOVE_DEAD_WEAKREF_METHODDEF \
39+
{"_remove_dead_weakref", (PyCFunction)_weakref__remove_dead_weakref, METH_VARARGS, _weakref__remove_dead_weakref__doc__},
40+
41+
static PyObject *
42+
_weakref__remove_dead_weakref_impl(PyObject *module, PyObject *dct,
43+
PyObject *key);
44+
45+
static PyObject *
46+
_weakref__remove_dead_weakref(PyObject *module, PyObject *args)
47+
{
48+
PyObject *return_value = NULL;
49+
PyObject *dct;
50+
PyObject *key;
51+
52+
if (!PyArg_ParseTuple(args, "O!O:_remove_dead_weakref",
53+
&PyDict_Type, &dct, &key))
54+
goto exit;
55+
return_value = _weakref__remove_dead_weakref_impl(module, dct, key);
56+
57+
exit:
58+
return return_value;
59+
}
60+
/*[clinic end generated code: output=5764cb64a6f66ffd input=a9049054013a1b77]*/

‎Objects/dictobject.c‎

Lines changed: 56 additions & 25 deletions

0 commit comments

Comments
 (0)