Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e10ca3a

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

File tree

Expand file treeCollapse file tree

7 files changed

+174
-30
lines changed
Filter options
Expand file treeCollapse file tree

7 files changed

+174
-30
lines changed

‎Include/dictobject.h

Copy file name to clipboardExpand all lines: Include/dictobject.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ PyAPI_FUNC(int) PyDict_DelItem(PyObject *mp, PyObject *key);
7575
#ifndef Py_LIMITED_API
7676
PyAPI_FUNC(int) _PyDict_DelItem_KnownHash(PyObject *mp, PyObject *key,
7777
Py_hash_t hash);
78+
PyAPI_FUNC(int) _PyDict_DelItemIf(PyObject *mp, PyObject *key,
79+
int (*predicate)(PyObject *value));
7880
#endif
7981
PyAPI_FUNC(void) PyDict_Clear(PyObject *mp);
8082
PyAPI_FUNC(int) PyDict_Next(

‎Lib/test/test_weakref.py

Copy file name to clipboardExpand all lines: Lib/test/test_weakref.py
+12Lines 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

Copy file name to clipboardExpand all lines: Lib/weakref.py
+30-4Lines 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

Copy file name to clipboardExpand all lines: Misc/NEWS
+3Lines 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

Copy file name to clipboardExpand all lines: Modules/_weakref.c
+41Lines 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

Copy file name to clipboardExpand all lines: Modules/clinic/_weakref.c.h
+30-1Lines 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

Copy file name to clipboardExpand all lines: Objects/dictobject.c
+56-25Lines changed: 56 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,13 +1246,31 @@ _PyDict_SetItem_KnownHash(PyObject *op, PyObject *key, PyObject *value,
12461246
return insertdict(mp, key, hash, value);
12471247
}
12481248

1249+
static int
1250+
delitem_common(PyDictObject *mp, PyDictKeyEntry *ep, PyObject **value_addr)
1251+
{
1252+
PyObject *old_key, *old_value;
1253+
1254+
old_value = *value_addr;
1255+
*value_addr = NULL;
1256+
mp->ma_used--;
1257+
if (!_PyDict_HasSplitTable(mp)) {
1258+
ENSURE_ALLOWS_DELETIONS(mp);
1259+
old_key = ep->me_key;
1260+
Py_INCREF(dummy);
1261+
ep->me_key = dummy;
1262+
Py_DECREF(old_key);
1263+
}
1264+
Py_DECREF(old_value);
1265+
return 0;
1266+
}
1267+
12491268
int
12501269
PyDict_DelItem(PyObject *op, PyObject *key)
12511270
{
12521271
PyDictObject *mp;
12531272
Py_hash_t hash;
12541273
PyDictKeyEntry *ep;
1255-
PyObject *old_key, *old_value;
12561274
PyObject **value_addr;
12571275

12581276
if (!PyDict_Check(op)) {
@@ -1274,26 +1292,14 @@ PyDict_DelItem(PyObject *op, PyObject *key)
12741292
_PyErr_SetKeyError(key);
12751293
return -1;
12761294
}
1277-
old_value = *value_addr;
1278-
*value_addr = NULL;
1279-
mp->ma_used--;
1280-
if (!_PyDict_HasSplitTable(mp)) {
1281-
ENSURE_ALLOWS_DELETIONS(mp);
1282-
old_key = ep->me_key;
1283-
Py_INCREF(dummy);
1284-
ep->me_key = dummy;
1285-
Py_DECREF(old_key);
1286-
}
1287-
Py_DECREF(old_value);
1288-
return 0;
1295+
return delitem_common(mp, ep, value_addr);
12891296
}
12901297

12911298
int
12921299
_PyDict_DelItem_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash)
12931300
{
12941301
PyDictObject *mp;
12951302
PyDictKeyEntry *ep;
1296-
PyObject *old_key, *old_value;
12971303
PyObject **value_addr;
12981304

12991305
if (!PyDict_Check(op)) {
@@ -1310,20 +1316,45 @@ _PyDict_DelItem_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash)
13101316
_PyErr_SetKeyError(key);
13111317
return -1;
13121318
}
1313-
old_value = *value_addr;
1314-
*value_addr = NULL;
1315-
mp->ma_used--;
1316-
if (!_PyDict_HasSplitTable(mp)) {
1317-
ENSURE_ALLOWS_DELETIONS(mp);
1318-
old_key = ep->me_key;
1319-
Py_INCREF(dummy);
1320-
ep->me_key = dummy;
1321-
Py_DECREF(old_key);
1319+
return delitem_common(mp, ep, value_addr);
1320+
}
1321+
1322+
int
1323+
_PyDict_DelItemIf(PyObject *op, PyObject *key,
1324+
int (*predicate)(PyObject *value))
1325+
{
1326+
PyDictObject *mp;
1327+
Py_hash_t hash;
1328+
PyDictKeyEntry *ep;
1329+
PyObject **value_addr;
1330+
int res;
1331+
1332+
if (!PyDict_Check(op)) {
1333+
PyErr_BadInternalCall();
1334+
return -1;
13221335
}
1323-
Py_DECREF(old_value);
1324-
return 0;
1336+
assert(key);
1337+
hash = PyObject_Hash(key);
1338+
if (hash == -1)
1339+
return -1;
1340+
mp = (PyDictObject *)op;
1341+
ep = (mp->ma_keys->dk_lookup)(mp, key, hash, &value_addr);
1342+
if (ep == NULL)
1343+
return -1;
1344+
if (*value_addr == NULL) {
1345+
_PyErr_SetKeyError(key);
1346+
return -1;
1347+
}
1348+
res = predicate(*value_addr);
1349+
if (res == -1)
1350+
return -1;
1351+
if (res > 0)
1352+
return delitem_common(mp, ep, value_addr);
1353+
else
1354+
return 0;
13251355
}
13261356

1357+
13271358
void
13281359
PyDict_Clear(PyObject *op)
13291360
{

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.