8000 Merge branch 'gh-111926-thread-safe-weakref' into nogil-integration · colesbury/cpython@1dad954 · GitHub
[go: up one dir, main page]
Skip to content

Commit 1dad954

Browse files
colesburycolesbury
committed
Merge branch 'pythongh-111926-thread-safe-weakref' into nogil-integration
2 parents 36cca5b + 8982831 commit 1dad954

File tree

12 files changed

+578
-188
lines changed

12 files changed

+578
-188
lines changed

‎Include/internal/pycore_interp.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ struct _stoptheworld_state {
5959
PyThreadState *requester; // Thread that requested the pause (may be NULL).
6060
};
6161

62+
#ifdef Py_GIL_DISABLED
63+
// This should be prime but otherwise the choice is arbitrary. A larger value
64+
// increases concurrency at the expense of memory.
65+
#define NUM_WEAKREF_LIST_LOCKS 127
66+
#endif
67+
6268
/* cross-interpreter data registry */
6369

6470
/* Tracks some rare events per-interpreter, used by the optimizer to turn on/off
@@ -203,6 +209,7 @@ struct _is {
203209
#if defined(Py_GIL_DISABLED)
204210
struct _mimalloc_interp_state mimalloc;
205211
struct _brc_state brc; // biased reference counting state
212+
PyMutex weakref_locks[NUM_WEAKREF_LIST_LOCKS];
206213
#endif
207214

208215
// Per-interpreter state for the obmalloc allocator. For the main

‎Include/internal/pycore_lock.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ typedef enum _PyLockFlags {
113113

114114
// Lock a mutex with an optional timeout and additional options. See
115115
// _PyLockFlags for details.
116-
extern PyLockStatus
116+
PyAPI_FUNC(PyLockStatus)
117117
_PyMutex_LockTimed(PyMutex *m, PyTime_t timeout_ns, _PyLockFlags flags);
118118

119119
// Lock a mutex with aditional options. See _PyLockFlags for details.

‎Include/internal/pycore_object.h

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ PyAPI_FUNC(void) _Py_NO_RETURN _Py_FatalRefcountErrorFunc(
8888
PyAPI_DATA(Py_ssize_t) _Py_RefTotal;
8989

9090
extern void _Py_AddRefTotal(PyInterpreterState *, Py_ssize_t);
91-
extern void _Py_IncRefTotal(PyInterpreterState *);
9291
extern void _Py_DecRefTotal(PyInterpreterState *);
9392

9493
# define _Py_DEC_REFTOTAL(interp) \
@@ -507,6 +506,25 @@ _Py_XNewRefWithLock(PyObject *obj)
507506
return _Py_NewRefWithLock(obj);
508507
}
509508

509+
static inline void
510+
_PyObject_SetMaybeWeakref(PyObject *op)
511+
{
512+
if (_Py_IsImmortal(op)) {
513+
return;
514+
}
515+
for (;;) {
516+
Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&op->ob_ref_shared);
517+
if ((shared & _Py_REF_SHARED_FLAG_MASK) != 0) {
518+
// Nothing to do if it's in WEAKREFS, QUEUED, or MERGED states.
519+
return;
520+
}
521+
if (_Py_atomic_compare_exchange_ssize(
522+
&op->ob_ref_shared, &shared, shared | _Py_REF_MAYBE_WEAKREF)) {
523+
return;
524+
}
525+
}
526+
}
527+
510528
#endif
511529

512530
#ifdef Py_REF_DEBUG

‎Include/internal/pycore_pyatomic_ft_wrappers.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ extern "C" {
3333
_Py_atomic_load_ptr_acquire(&value)
3434
#define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) \
3535
_Py_atomic_load_uintptr_acquire(&value)
36+
#define FT_ATOMIC_STORE_PTR(value, new_value) \
37+
_Py_atomic_store_ptr(&value, new_value)
3638
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
3739
_Py_atomic_store_ptr_relaxed(&value, new_value)
3840
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \
@@ -51,6 +53,7 @@ extern "C" {
5153
#define FT_ATOMIC_LOAD_UINT8_RELAXED(value) value
5254
#define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) value
5355
#define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) value
56+
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
5457
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
5558
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
5659
#define FT_ATOMIC_STORE_UINTPTR_RELEASE(value, new_value) value = new_value

‎Include/internal/pycore_weakref.h

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,26 @@ extern "C" {
99
#endif
1010

1111
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION()
12+
#include "pycore_lock.h"
1213
#include "pycore_object.h" // _Py_REF_IS_MERGED()
1314

15+
#ifdef Py_GIL_DISABLED
16+
17+
#define WEAKREF_LIST_LOCK(obj) \
18+
_PyInterpreterState_GET() \
19+
->weakref_locks[((uintptr_t)obj) % NUM_WEAKREF_LIST_LOCKS]
20+
21+
#define LOCK_WEAKREFS(obj) \
22+
PyMutex_LockFlags(&WEAKREF_LIST_LOCK(obj), _Py_LOCK_DONT_DETACH)
23+
#define UNLOCK_WEAKREFS(obj) PyMutex_Unlock(&WEAKREF_LIST_LOCK(obj))
24+
25+
#else
26+
27+
#define LOCK_WEAKREFS(obj)
28+
#define UNLOCK_WEAKREFS(obj)
29+
30+
#endif
31+
1432
static inline int _is_dead(PyObject *obj)
1533
{
1634
// Explanation for the Py_REFCNT() check: when a weakref's target is part
@@ -30,53 +48,82 @@ static inline int _is_dead(PyObject *obj)
3048
static inline PyObject* _PyWeakref_GET_REF(PyObject *ref_obj)
3149
{
3250
assert(PyWeakref_Check(ref_obj));
33-
PyObject *ret = NULL;
34-
Py_BEGIN_CRITICAL_SECTION(ref_obj);
3551
PyWeakReference *ref = _Py_CAST(PyWeakReference*, ref_obj);
36-
PyObject *obj = ref->wr_object;
3752

53+
#if !defined(Py_GIL_DISABLED)
54+
PyObject *obj = ref->wr_object;
3855
if (obj == Py_None) {
3956
// clear_weakref() was called
40-
goto end;
57+
return NULL;
4158
}
4259

4360
if (_is_dead(obj)) {
44-
goto end;
61+
return NULL;
4562
}
46-
#if !defined(Py_GIL_DISABLED)
4763
assert(Py_REFCNT(obj) > 0);
64+
return Py_NewRef(obj);
65+
#else
66+
PyObject *obj = _Py_atomic_load_ptr(&ref->wr_object);
67+
if (obj == Py_None) {
68+
// clear_weakref() was called
69+
return NULL;
70+
}
71+
LOCK_WEAKREFS(obj);
72+
if (_Py_atomic_load_ptr(&ref->wr_object) == Py_None) {
73+
// clear_weakref() was called
74+
UNLOCK_WEAKREFS(obj);
75+
return NULL;
76+
}
77+
if (_Py_TryIncref(&obj, obj)) {
78+
UNLOCK_WEAKREFS(obj);
79+
return obj;
80+
}
81+
UNLOCK_WEAKREFS(obj);
82+
return NULL;
4883
#endif
49-
ret = Py_NewRef(obj);
50-
end:
51-
Py_END_CRITICAL_SECTION();
52-
return ret;
5384
}
5485

5586
static inline int _PyWeakref_IS_DEAD(PyObject *ref_obj)
5687
{
5788
assert(PyWeakref_Check(ref_obj));
5889
int ret = 0;
59-
Py_BEGIN_CRITICAL_SECTION(ref_obj);
6090
PyWeakReference *ref = _Py_CAST(PyWeakReference*, ref_obj);
91+
#ifdef Py_GIL_DISABLED
92+
PyObject *obj = _Py_atomic_load_ptr(&ref->wr_object);
93+
#else
6194
PyObject *obj = ref->wr_object;
95+
#endif
6296
if (obj == Py_None) {
6397
// clear_weakref() was called
6498
ret = 1;
6599
}
66100
else {
101+
LOCK_WEAKREFS(obj);
67102
// See _PyWeakref_GET_REF() for the rationale of this test
103+
#ifdef Py_GIL_DISABLED
104+
PyObject *obj_reloaded = _Py_atomic_load_ptr(&ref->wr_object);
105+
ret = (obj_reloaded == Py_None) || _is_dead(obj);
106+
#else
68107
ret = _is_dead(obj);
108+
#endif
109+
UNLOCK_WEAKREFS(obj);
69110
}
70-
Py_END_CRITICAL_SECTION();
71111
return ret;
72112
}
73113

114+
// NB: In free-threaded builds the weakref list lock for the referenced object
115+
// must be held around calls to this function.
74116
extern Py_ssize_t _PyWeakref_GetWeakrefCount(PyWeakReference *head);
75117

118+
extern Py_ssize_t _PyWeakref_GetWeakrefCountThreadsafe(PyObject *obj);
119+
120+
// Clear all the weak references to obj but leave their callbacks uncalled and
121+
// intact.
122+
extern void _PyWeakref_ClearWeakRefsExceptCallbacks(PyObject *obj);
123+
76124
extern void _PyWeakref_ClearRef(PyWeakReference *self);
77125

78126
#ifdef __cplusplus
79127
}
80128
#endif
81129
#endif /* !Py_INTERNAL_WEAKREF_H */
82-

‎Include/object.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,7 @@ PyAPI_FUNC(void) _Py_NegativeRefcount(const char *filename, int lineno,
771771
PyObject *op);
772772
PyAPI_FUNC(void) _Py_INCREF_IncRefTotal(void);
773773
PyAPI_FUNC(void) _Py_DECREF_DecRefTotal(void);
774+
PyAPI_FUNC(void) _Py_IncRefTotal(PyInterpreterState *);
774775
#endif // Py_REF_DEBUG && !Py_LIMITED_API
775776

776777
PyAPI_FUNC(void) _Py_Dealloc(PyObject *);

‎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: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ module _weakref
1414
#include "clinic/_weakref.c.h"
1515

1616
/*[clinic input]
17-
@critical_section object
1817
_weakref.getweakrefcount -> Py_ssize_t
1918
2019
object: object
@@ -25,14 +24,12 @@ Return the number of weak references to 'object'.
2524

2625
static Py_ssize_t
2726
_weakref_getweakrefcount_impl(PyObject *module, PyObject *object)
28-
/*[clinic end generated code: output=301806d59558ff3e input=6535a580f1d0ebdc]*/
27+
/*[clinic end generated code: output=301806d59558ff3e input=7d4d04fcaccf64d5]*/
2928
{
3029
if (!_PyType_SUPPORTS_WEAKREFS(Py_TYPE(object))) {
3130
return 0;
3231
}
33-
PyWeakReference **list = GET_WEAKREFS_LISTPTR(object);
34-
Py_ssize_t count = _PyWeakref_GetWeakrefCount(*list);
35-
return count;
32+
return _PyWeakref_GetWeakrefCountThreadsafe(object);
3633
}
3734

3835

@@ -77,7 +74,6 @@ _weakref__remove_dead_weakref_impl(PyObject *module, PyObject *dct,
7774

7875

7976
/*[clinic input]
80-
@critical_section object
8177
_weakref.getweakrefs
8278
object: object
8379
/
@@ -86,26 +82,57 @@ Return a list of all weak reference objects pointing to 'object'.
8682
[clinic start generated code]*/
8783

8884
static PyObject *
89-
_weakref_getweakrefs_impl(PyObject *module, PyObject *object)
90-
/*[clinic end generated code: output=5ec268989fb8f035 input=3dea95b8f5b31bbb]*/
85+
_weakref_getweakrefs(PyObject *module, PyObject *object)
86+
/*[clinic end generated code: output=25c7731d8e011824 input=00c6d0e5d3206693]*/
9187
{
9288
if (!_PyType_SUPPORTS_WEAKREFS(Py_TYPE(object))) {
9389
return PyList_New(0);
9490
}
9591

9692
PyWeakReference **list = GET_WEAKREFS_LISTPTR(object);
97-
Py_ssize_t count = _PyWeakref_GetWeakrefCount(*list);
93+
Py_ssize_t count = _PyWeakref_GetWeakrefCountThreadsafe(object);
9894

9995
PyObject *result = PyList_New(count);
10096
if (result == NULL) {
10197
return NULL;
10298
}
10399

100+
#ifdef Py_GIL_DISABLED
101+
Py_ssize_t num_added = 0;
102+
LOCK_WEAKREFS(object);
103+
PyWeakReference *current = *list;
104+
// Weakrefs may be added or removed since the count was computed.
105+
while (num_added < count && current != NULL) {
106+
if (_Py_TryIncref((PyObject**) &current, (PyObject *) current)) {
107+
PyList_SET_ITEM(result, num_added, current);
108+
num_added++;
109+
}
110+
current = current->wr_next;
111+
}
112+
UNLOCK_WEAKREFS(object);
113+
114+
// Don't return an incomplete list
115+
if (num_added != count) {
116+
PyObject *new_list = PyList_New(num_added);
117+
if (new_list == NULL) {
118+
Py_DECREF(result);
119+
return NULL;
120+
}
121+
for (Py_ssize_t i = 0; i < num_added; i++) {
122+
PyObject *obj = PyList_GET_ITEM(result, i);
123+
PyList_SET_ITEM(new_list, i, obj);
124+
PyList_SET_ITEM(result, i, NULL);
125+
}
126+
Py_DECREF(result);
127+
result = new_list;
128+
}
129+
#else
104130
PyWeakReference *current = *list;
105131
for (Py_ssize_t i = 0; i < count; ++i) {
106132
PyList_SET_ITEM(result, i, Py_NewRef(current));
107133
current = current->wr_next;
108134
}
135+
#endif
109136
return result;
110137
}
111138

‎Modules/clinic/_weakref.c.h

Lines changed: 1 addition & 19 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
0