traverseda
/
cpython
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Issue #28427: old keys should not remove new values from 

WeakValueDictionary when collecting from another thread.
This commit is contained in:
Antoine Pitrou 2016-12-27 14:34:54 +01:00
parent a171a03bd2 d741ed492f
commit c06ae208eb
7 changed files with 194 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Include/dictobject.h
View File

@ -88,6 +88,8 @@ PyAPI_FUNC(int) PyDict_DelItem(PyObject *mp, PyObject *key);
#ifndef Py_LIMITED_API
PyAPI_FUNC(int) _PyDict_DelItem_KnownHash(PyObject *mp, PyObject *key,
Py_hash_t hash);
PyAPI_FUNC(int) _PyDict_DelItemIf(PyObject *mp, PyObject *key,
int (*predicate)(PyObject *value));
#endif
PyAPI_FUNC(void) PyDict_Clear(PyObject *mp);
PyAPI_FUNC(int) PyDict_Next(

12
Lib/test/test_weakref.py
View File

@ -1676,6 +1676,18 @@ class MappingTestCase(TestBase):
x = d.pop(10, 10)
self.assertIsNot(x, None) # we never put None in there!
def test_threaded_weak_valued_consistency(self):
# Issue #28427: old keys should not remove new values from
# WeakValueDictionary when collecting from another thread.
d = weakref.WeakValueDictionary()
with collect_in_thread():
for i in range(200000):
o = RefCycle()
d[10] = o
# o is still alive, so the dict can't be empty
self.assertEqual(len(d), 1)
o = None # lose ref
from test import mapping_tests

34
Lib/weakref.py
View File

@ -16,7 +16,8 @@ from _weakref import (
proxy,
CallableProxyType,
ProxyType,
ReferenceType)
ReferenceType,
_remove_dead_weakref)
from _weakrefset import WeakSet, _IterationGuard
@ -111,7 +112,9 @@ class WeakValueDictionary(collections.MutableMapping):
if self._iterating:
self._pending_removals.append(wr.key)
else:
del self.data[wr.key]
# Atomic removal is necessary since this function
# can be called asynchronously by the GC
_remove_dead_weakref(d, wr.key)
self._remove = remove
# A list of keys to be removed
self._pending_removals = []
@ -125,9 +128,12 @@ class WeakValueDictionary(collections.MutableMapping):
# We shouldn't encounter any KeyError, because this method should
# always be called *before* mutating the dict.
while l:
del d[l.pop()]
key = l.pop()
_remove_dead_weakref(d, key)
def __getitem__(self, key):
if self._pending_removals:
self._commit_removals()
o = self.data[key]()
if o is None:
raise KeyError(key)
@ -140,9 +146,13 @@ class WeakValueDictionary(collections.MutableMapping):
del self.data[key]
def __len__(self):
return len(self.data) - len(self._pending_removals)
if self._pending_removals:
self._commit_removals()
return len(self.data)
def __contains__(self, key):
if self._pending_removals:
self._commit_removals()
try:
o = self.data[key]()
except KeyError:
@ -158,6 +168,8 @@ class WeakValueDictionary(collections.MutableMapping):
self.data[key] = KeyedRef(value, self._remove, key)
def copy(self):
if self._pending_removals:
self._commit_removals()
new = WeakValueDictionary()
for key, wr in self.data.items():
o = wr()
@ -169,6 +181,8 @@ class WeakValueDictionary(collections.MutableMapping):
def __deepcopy__(self, memo):
from copy import deepcopy
if self._pending_removals:
self._commit_removals()
new = self.__class__()
for key, wr in self.data.items():
o = wr()
@ -177,6 +191,8 @@ class WeakValueDictionary(collections.MutableMapping):
return new
def get(self, key, default=None):
if self._pending_removals:
self._commit_removals()
try:
wr = self.data[key]
except KeyError:
@ -190,6 +206,8 @@ class WeakValueDictionary(collections.MutableMapping):
return o
def items(self):
if self._pending_removals:
self._commit_removals()
with _IterationGuard(self):
for k, wr in self.data.items():
v = wr()
@ -197,6 +215,8 @@ class WeakValueDictionary(collections.MutableMapping):
yield k, v
def keys(self):
if self._pending_removals:
self._commit_removals()
with _IterationGuard(self):
for k, wr in self.data.items():
if wr() is not None:
@ -214,10 +234,14 @@ class WeakValueDictionary(collections.MutableMapping):
keep the values around longer than needed.
"""
if self._pending_removals:
self._commit_removals()
with _IterationGuard(self):
yield from self.data.values()
def values(self):
if self._pending_removals:
self._commit_removals()
with _IterationGuard(self):
for wr in self.data.values():
obj = wr()
@ -290,6 +314,8 @@ class WeakValueDictionary(collections.MutableMapping):
keep the values around longer than needed.
"""
if self._pending_removals:
self._commit_removals()
return list(self.data.values())

3
Misc/NEWS
View File

@ -208,6 +208,9 @@ Core and Builtins
Library
-------
- Issue #28427: old keys should not remove new values from
WeakValueDictionary when collecting from another thread.
- Issue 28923: Remove editor artifacts from Tix.py.
- Issue #28871: Fixed a crash when deallocate deep ElementTree.

41
Modules/_weakref.c
View File

@ -35,6 +35,46 @@ _weakref_getweakrefcount_impl(PyObject *module, PyObject *object)
}
static int
is_dead_weakref(PyObject *value)
{
if (!PyWeakref_Check(value)) {
PyErr_SetString(PyExc_TypeError, "not a weakref");
return -1;
}
return PyWeakref_GET_OBJECT(value) == Py_None;
}
/*[clinic input]
_weakref._remove_dead_weakref -> object
dct: object(subclass_of='&PyDict_Type')
key: object
/
Atomically remove key from dict if it points to a dead weakref.
[clinic start generated code]*/
static PyObject *
_weakref__remove_dead_weakref_impl(PyObject *module, PyObject *dct,
PyObject *key)
/*[clinic end generated code: output=d9ff53061fcb875c input=19fc91f257f96a1d]*/
{
if (_PyDict_DelItemIf(dct, key, is_dead_weakref) < 0) {
if (PyErr_ExceptionMatches(PyExc_KeyError))
/* This function is meant to allow safe weak-value dicts
with GC in another thread (see issue #28427), so it's
ok if the key doesn't exist anymore.
*/
PyErr_Clear();
else
return NULL;
}
Py_RETURN_NONE;
}
PyDoc_STRVAR(weakref_getweakrefs__doc__,
"getweakrefs(object) -- return a list of all weak reference objects\n"
"that point to 'object'.");
@ -88,6 +128,7 @@ weakref_proxy(PyObject *self, PyObject *args)
static PyMethodDef
weakref_functions[] = {
_WEAKREF_GETWEAKREFCOUNT_METHODDEF
_WEAKREF__REMOVE_DEAD_WEAKREF_METHODDEF
{"getweakrefs", weakref_getweakrefs, METH_O,
weakref_getweakrefs__doc__},
{"proxy", weakref_proxy, METH_VARARGS,

32
Modules/clinic/_weakref.c.h
View File

@ -29,4 +29,34 @@ _weakref_getweakrefcount(PyObject *module, PyObject *object)
exit:
return return_value;
}
/*[clinic end generated code: output=e1ad587147323e19 input=a9049054013a1b77]*/
PyDoc_STRVAR(_weakref__remove_dead_weakref__doc__,
"_remove_dead_weakref($module, dct, key, /)\n"
"--\n"
"\n"
"Atomically remove key from dict if it points to a dead weakref.");
#define _WEAKREF__REMOVE_DEAD_WEAKREF_METHODDEF \
{"_remove_dead_weakref", (PyCFunction)_weakref__remove_dead_weakref, METH_VARARGS, _weakref__remove_dead_weakref__doc__},
static PyObject *
_weakref__remove_dead_weakref_impl(PyObject *module, PyObject *dct,
PyObject *key);
static PyObject *
_weakref__remove_dead_weakref(PyObject *module, PyObject *args)
{
PyObject *return_value = NULL;
PyObject *dct;
PyObject *key;
if (!PyArg_ParseTuple(args, "O!O:_remove_dead_weakref",
&PyDict_Type, &dct, &key)) {
goto exit;
}
return_value = _weakref__remove_dead_weakref_impl(module, dct, key);
exit:
return return_value;
}
/*[clinic end generated code: output=e860dd818a44bc9b input=a9049054013a1b77]*/

92
Objects/dictobject.c
View File

@ -1583,11 +1583,32 @@ _PyDict_SetItem_KnownHash(PyObject *op, PyObject *key, PyObject *value,
return insertdict(mp, key, hash, value);
}
static int
delitem_common(PyDictObject *mp, Py_ssize_t hashpos, Py_ssize_t ix,
PyObject *old_value)
{
PyObject *old_key;
PyDictKeyEntry *ep;
mp->ma_used--;
mp->ma_version_tag = DICT_NEXT_VERSION();
ep = &DK_ENTRIES(mp->ma_keys)[ix];
dk_set_index(mp->ma_keys, hashpos, DKIX_DUMMY);
ENSURE_ALLOWS_DELETIONS(mp);
old_key = ep->me_key;
ep->me_key = NULL;
ep->me_value = NULL;
Py_DECREF(old_key);
Py_DECREF(old_value);
assert(_PyDict_CheckConsistency(mp));
return 0;
}
int
PyDict_DelItem(PyObject *op, PyObject *key)
{
Py_hash_t hash;
assert(key);
if (!PyUnicode_CheckExact(key) ||
(hash = ((PyASCIIObject *) key)->hash) == -1) {
@ -1604,8 +1625,7 @@ _PyDict_DelItem_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash)
{
Py_ssize_t hashpos, ix;
PyDictObject *mp;
PyDictKeyEntry *ep;
PyObject *old_key, *old_value;
PyObject *old_value;
if (!PyDict_Check(op)) {
PyErr_BadInternalCall();
@ -1632,22 +1652,60 @@ _PyDict_DelItem_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash)
assert(ix >= 0);
}
assert(old_value != NULL);
mp->ma_used--;
mp->ma_version_tag = DICT_NEXT_VERSION();
ep = &DK_ENTRIES(mp->ma_keys)[ix];
dk_set_index(mp->ma_keys, hashpos, DKIX_DUMMY);
ENSURE_ALLOWS_DELETIONS(mp);
old_key = ep->me_key;
ep->me_key = NULL;
ep->me_value = NULL;
Py_DECREF(old_key);
Py_DECREF(old_value);
assert(_PyDict_CheckConsistency(mp));
return 0;
return delitem_common(mp, hashpos, ix, old_value);
}
/* This function promises that the predicate -> deletion sequence is atomic
* (i.e. protected by the GIL), assuming the predicate itself doesn't
* release the GIL.
*/
int
_PyDict_DelItemIf(PyObject *op, PyObject *key,
int (*predicate)(PyObject *value))
{
Py_ssize_t hashpos, ix;
PyDictObject *mp;
Py_hash_t hash;
PyObject *old_value;
int res;
if (!PyDict_Check(op)) {
PyErr_BadInternalCall();
return -1;
}
assert(key);
hash = PyObject_Hash(key);
if (hash == -1)
return -1;
mp = (PyDictObject *)op;
ix = (mp->ma_keys->dk_lookup)(mp, key, hash, &old_value, &hashpos);
if (ix == DKIX_ERROR)
return -1;
if (ix == DKIX_EMPTY || old_value == NULL) {
_PyErr_SetKeyError(key);
return -1;
}
assert(dk_get_index(mp->ma_keys, hashpos) == ix);
// Split table doesn't allow deletion. Combine it.
if (_PyDict_HasSplitTable(mp)) {
if (dictresize(mp, DK_SIZE(mp->ma_keys))) {
return -1;
}
ix = (mp->ma_keys->dk_lookup)(mp, key, hash, &old_value, &hashpos);
assert(ix >= 0);
}
res = predicate(old_value);
if (res == -1)
return -1;
if (res > 0)
return delitem_common(mp, hashpos, ix, old_value);
else
return 0;
}
void
PyDict_Clear(PyObject *op)
{