8000 Support async removal of dead weakrefs from WeakValueDictionary · python/cpython@a07d2db · GitHub
[go: up one dir, main page]
Skip to content

Commit a07d2db

Browse files
mpagempage
committed
Support async removal of dead weakrefs from WeakValueDictionary
1 parent 37076b5 commit a07d2db

File tree

4 files changed

+112
-0
lines changed

4 files changed

+112
-0
lines changed

‎Include/internal/pycore_dict.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ extern PyObject* _PyDict_GetItemWithError(PyObject *dp, PyObject *key);
1818

1919
extern int _PyDict_DelItemIf(PyObject *mp, PyObject *key,
2020
int (*predicate)(PyObject *value));
21+
#ifdef Py_GIL_DISABLED
22+
extern int _PyDict_DelItemIfDeadWeakref(PyObject *mp, PyObject *key);
23+
#endif
2124

2225
// "KnownHash" variants
2326
// Export for '_asyncio' shared extension

‎Lib/test/test_weakref.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1865,6 +1865,25 @@ def test_threaded_weak_valued_consistency(self):
18651865
self.assertEqual(len(d), 1)
18661866
o = None # lose ref
18671867

1868+
@support.cpython_only
1869+
def test_weak_valued_consistency(self):
1870+
# A single-threaded, deterministic repro for issue #28427: old keys
1871+
# should not remove new values from WeakValueDictionary. This relies on
1872+
# an implementation detail of CPython's WeakValueDictionary (its
1873+
# underlying dictionary of KeyedRefs) to reproduce the issue.
1874+
d = weakref.WeakValueDictionary()
1875+
with support.disable_gc():
1876+
d[10] = RefCycle()
1877+
# Keep the KeyedRef alive after it's replaced so that GC will invoke
1878+
# the callback.
1879+
wr = d.data[10]
1880+
# Replace the value with something that isn't cyclic garbage
1881+
o = RefCycle()
1882+
d[10] = o
1883+
# Trigger GC, which will invoke the callback for `wr`
1884+
gc.collect()
1885+
self.assertEqual(len(d), 1)
1886+
18681887
def check_threaded_weak_dict_copy(self, type_, deepcopy):
18691888
# `type_` should be either WeakKeyDictionary or WeakValueDictionary.
18701889
# `deepcopy` should be either True or False.

‎Modules/_weakref.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ _weakref_getweakrefcount_impl(PyObject *module, PyObject *object)
3232
return _PyWeakref_GetWeakrefCountThreadsafe(object);
3333
}
3434

35+
#ifndef Py_GIL_DISABLED
3536

3637
static int
3738
is_dead_weakref(PyObject *value)
@@ -43,6 +44,8 @@ is_dead_weakref(PyObject *value)
4344
return _PyWeakref_IS_DEAD(value);
4445
}
4546

47+
#endif
48+
4649
/*[clinic input]
4750
4851
_weakref._remove_dead_weakref -> object
@@ -59,6 +62,11 @@ _weakref__remove_dead_weakref_impl(PyObject *module, PyObject *dct,
5962
PyObject *key)
6063
/*[clinic end generated code: output=d9ff53061fcb875c input=19fc91f257f96a1d]*/
6164
{
65+
#ifdef Py_GIL_DISABLED
66+
if (_PyDict_DelItemIfDeadWeakref(dct, key) < 0) {
67+
return NULL;
68+
}
69+
#else
6270
if (_PyDict_DelItemIf(dct, key, is_dead_weakref) < 0) {
6371
if (PyErr_ExceptionMatches(PyExc_KeyError))
6472
/* This function is meant to allow safe weak-value dicts
@@ -69,6 +77,7 @@ _weakref__remove_dead_weakref_impl(PyObject *module, PyObject *dct,
6977
else
7078
return NULL;
7179
}
80+
#endif
7281
Py_RETURN_NONE;
7382
}
7483

‎Objects/dictobject.c

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ As a consequence of this, split keys have a maximum size of 16.
129129
#include "pycore_pyerrors.h" // _PyErr_GetRaisedException()
130130
#include "pycore_pystate.h" // _PyThreadState_GET()
131131
#include "pycore_setobject.h" // _PySet_NextEntry()
132+
#include "pycore_weakref.h"
132133
#include "stringlib/eq.h" // unicode_eq()
133134

134135
#include <stdbool.h>
@@ -2638,6 +2639,86 @@ _PyDict_DelItemIf(PyObject *op, PyObject *key,
26382639
return res;
26392640
}
26402641

2642+
#ifdef Py_GIL_DISABLED
2643+
2644+
static int
2645+
delitemifdeadweakref_lock_held(PyObject *op, PyObject *key)
2646+
{
2647+
Py_ssize_t hashpos, ix;
2648+
PyDictObject *mp;
2649+
Py_hash_t hash;
2650+
PyObject *old_value;
2651+
2652+
ASSERT_DICT_LOCKED(op);
2653+
2654+
if (!PyDict_Check(op)) {
2655+
PyErr_BadInternalCall();
2656+
return -1;
2657+
}
2658+
assert(key);
2659+
hash = PyObject_Hash(key);
2660+
if (hash == -1)
2661+
return -1;
2662+
mp = (PyDictObject *)op;
2663+
ix = _Py_dict_lookup(mp, key, hash, &old_value);
2664+
if (ix == DKIX_ERROR)
2665+
return -1;
2666+
if (ix == DKIX_EMPTY || old_value == NULL) {
2667+
return 0;
2668+
}
2669+
2670+
// PyWeakref_Check may suspend; we need to get a strong reference
2671+
// to old_value in case its removed from the dictionary while
2672+
// we're suspended.
2673+
PyObject *orig_old_value = Py_NewRef(old_value);
2674+
if (!PyWeakref_Check(old_value)) {
2675+
PyErr_SetString(PyExc_TypeError, "not a weakref");
2676+
return -1;
2677+
}
2678+
2679+
if (!_PyWeakref_IS_DEAD(orig_old_value)) {
2680+
Py_DECREF(orig_old_value);
2681+
return 0;
2682+
}
2683+
2684+
// The call to _PyWeakref_IS_DEAD may have suspended and the original value
2685+
// removed.
2686+
ix = _Py_dict_lookup(mp, key, hash, &old_value);
2687+
if (ix == DKIX_ERROR) {
2688+
Py_DECREF(orig_old_value);
2689+
return -1;
2690+
}
2691+
if (ix == DKIX_EMPTY || old_value != orig_old_value) {
2692+
// The key was removed or a new value was inserted. If the new value
2693+
// points to a dead weakref, then a subsequent call to this function
2694+
// will remove it.
2695+
Py_DECREF(orig_old_value);
2696+
return 0;
2697+
}
2698+
2699+
hashpos = lookdict_index(mp->ma_keys, hash, ix);
2700+
assert(hashpos >= 0);
2701+
2702+
PyInterpreterState *interp = _PyInterpreterState_GET();
2703+
uint64_t new_version = _PyDict_NotifyEvent(
2704+
interp, PyDict_EVENT_DELETED, mp, key, NULL);
2705+
int res = delitem_common(mp, hashpos, ix, old_value, new_version);
2706+
Py_DECREF(orig_old_value);
2707+
return res;
2708+
}
2709+
2710+
int
2711+
_PyDict_DelItemIfDeadWeakref(PyObject *op, PyObject *key)
2712+
{
2713+
int res;
2714+
Py_BEGIN_CRITICAL_SECTION(op);
2715+
res = delitemifdeadweakref_lock_held(op, key);
2716+
Py_END_CRITICAL_SECTION();
2717+
return res;
2718+
}
2719+
2720+
#endif
2721+
26412722
static void
26422723
clear_lock_held(PyObject *op)
26432724
{

0 commit comments

Comments
 (0)
0