pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/python/cpython/commit/df7317904849a41d51db39d92c5d431a18e22637

41e157469407.css" /> gh-111926: Make weakrefs thread-safe in free-threaded builds (#117168) · python/cpython@df73179 · GitHub
Skip to content

Commit df73179

Browse files
mpagempage
authored
gh-111926: Make weakrefs thread-safe in free-threaded builds (#117168)
Most mutable data is protected by a striped lock that is keyed on the referenced object's address. The weakref's hash is protected using the weakref's per-object lock. Note that this only affects free-threaded builds. Apart from some minor refactoring, the added code is all either gated by `ifdef`s or is a no-op (e.g. `Py_BEGIN_CRITICAL_SECTION`).
1 parent e16062d commit df73179

File tree

17 files changed

+491
-327
lines changed

17 files changed

+491
-327
lines changed

‎Include/cpython/weakrefobject.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ struct _PyWeakReference {
3030
PyWeakReference *wr_prev;
3131
PyWeakReference *wr_next;
3232
vectorcallfunc vectorcall;
33+
34+
#ifdef Py_GIL_DISABLED
35+
/* Pointer to the lock used when clearing in free-threaded builds.
36+
* Normally this can be derived from wr_object, but in some cases we need
37+
* to lock after wr_object has been set to Py_None.
38+
*/
39+
struct _PyMutex *weakrefs_lock;
40+
#endif
3341
};
3442

3543
Py_DEPRECATED(3.13) static inline PyObject* PyWeakref_GET_OBJECT(PyObject *ref_obj)

‎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_object.h‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ _Py_TryIncRefShared(PyObject *op)
426426

427427
/* Tries to incref the object op and ensures that *src still points to it. */
428428
static inline int
429-
_Py_TryIncref(PyObject **src, PyObject *op)
429+
_Py_TryIncrefCompare(PyObject **src, PyObject *op)
430430
{
431431
if (_Py_TryIncrefFast(op)) {
432432
return 1;
@@ -452,7 +452,7 @@ _Py_XGetRef(PyObject **ptr)
452452
if (value == NULL) {
453453
return value;
454454
}
455-
if (_Py_TryIncref(ptr, value)) {
455+
if (_Py_TryIncrefCompare(ptr, value)) {
456456
return value;
457457
}
458458
}
@@ -467,7 +467,7 @@ _Py_TryXGetRef(PyObject **ptr)
467467
if (value == NULL) {
468468
return value;
469469
}
470-
if (_Py_TryIncref(ptr, value)) {
470+
if (_Py_TryIncrefCompare(ptr, value)) {
471471
return value;
472472
}
473473
return NULL;
@@ -506,8 +506,42 @@ _Py_XNewRefWithLock(PyObject *obj)
506506
return _Py_NewRefWithLock(obj);
507507
}
508508

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+
509528
#endif
510529

530+
/* Tries to incref op and returns 1 if successful or 0 otherwise. */
531+
static inline int
532+
_Py_TryIncref(PyObject *op)
533+
{
534+
#ifdef Py_GIL_DISABLED
535+
return _Py_TryIncrefFast(op) || _Py_TryIncRefShared(op);
536+
#else
537+
if (Py_REFCNT(op) > 0) {
538+
Py_INCREF(op);
539+
return 1;
540+
}
541+
return 0;
542+
#endif
543+
}
544+
511545
#ifdef Py_REF_DEBUG
512546
extern void _PyInterpreterState_FinalizeRefTotal(PyInterpreterState *);
513547
extern void _Py_FinalizeRefTotal(_PyRuntimeState *);

‎Include/internal/pycore_pyatomic_ft_wrappers.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,23 @@ extern "C" {
2020
#endif
2121

2222
#ifdef Py_GIL_DISABLED
23+
#define FT_ATOMIC_LOAD_PTR(value) _Py_atomic_load_ptr(&value)
2324
#define FT_ATOMIC_LOAD_SSIZE(value) _Py_atomic_load_ssize(&value)
2425
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) \
2526
_Py_atomic_load_ssize_relaxed(&value)
27+
#define FT_ATOMIC_STORE_PTR(value, new_value) \
28+
_Py_atomic_store_ptr(&value, new_value)
2629
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
2730
_Py_atomic_store_ptr_relaxed(&value, new_value)
2831
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \
2932
_Py_atomic_store_ptr_release(&value, new_value)
3033
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \
3134
_Py_atomic_store_ssize_relaxed(&value, new_value)
3235
#else
36+
#define FT_ATOMIC_LOAD_PTR(value) value
3337
#define FT_ATOMIC_LOAD_SSIZE(value) value
3438
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value
39+
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
3540
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
3641
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
3742
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value

‎Include/internal/pycore_weakref.h‎

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,35 @@ 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()
14+
#include "pycore_pyatomic_ft_wrappers.h"
15+
16+
#ifdef Py_GIL_DISABLED
17+
18+
#define WEAKREF_LIST_LOCK(obj) \
19+
_PyInterpreterState_GET() \
20+
->weakref_locks[((uintptr_t)obj) % NUM_WEAKREF_LIST_LOCKS]
21+
22+
// Lock using the referenced object
23+
#define LOCK_WEAKREFS(obj) \
24+
PyMutex_LockFlags(&WEAKREF_LIST_LOCK(obj), _Py_LOCK_DONT_DETACH)
25+
#define UNLOCK_WEAKREFS(obj) PyMutex_Unlock(&WEAKREF_LIST_LOCK(obj))
26+
27+
// Lock using a weakref
28+
#define LOCK_WEAKREFS_FOR_WR(wr) \
29+
PyMutex_LockFlags(wr->weakrefs_lock, _Py_LOCK_DONT_DETACH)
30+
#define UNLOCK_WEAKREFS_FOR_WR(wr) PyMutex_Unlock(wr->weakrefs_lock)
31+
32+
#else
33+
34+
#define LOCK_WEAKREFS(obj)
35+
#define UNLOCK_WEAKREFS(obj)
36+
37+
#define LOCK_WEAKREFS_FOR_WR(wr)
38+
#define UNLOCK_WEAKREFS_FOR_WR(wr)
39+
40+
#endif
1341

1442
static inline int _is_dead(PyObject *obj)
1543
{
@@ -30,53 +58,64 @@ static inline int _is_dead(PyObject *obj)
3058
static inline PyObject* _PyWeakref_GET_REF(PyObject *ref_obj)
3159
{
3260
assert(PyWeakref_Check(ref_obj));
33-
PyObject *ret = NULL;
34-
Py_BEGIN_CRITICAL_SECTION(ref_obj);
3561
PyWeakReference *ref = _Py_CAST(PyWeakReference*, ref_obj);
36-
PyObject *obj = ref->wr_object;
3762

63+
PyObject *obj = FT_ATOMIC_LOAD_PTR(ref->wr_object);
3864
if (obj == Py_None) {
3965
// clear_weakref() was called
40-
goto end;
66+
return NULL;
4167
}
4268

43-
if (_is_dead(obj)) {
44-
goto end;
69+
LOCK_WEAKREFS(obj);
70+
#ifdef Py_GIL_DISABLED
71+
if (ref->wr_object == Py_None) {
72+
// clear_weakref() was called
73+
UNLOCK_WEAKREFS(obj);
74+
return NULL;
4575
}
46-
#if !defined(Py_GIL_DISABLED)
47-
assert(Py_REFCNT(obj) > 0);
4876
#endif
49-
ret = Py_NewRef(obj);
50-
end:
51-
Py_END_CRITICAL_SECTION();
52-
return ret;
77+
if (_Py_TryIncref(obj)) {
78+
UNLOCK_WEAKREFS(obj);
79+
return obj;
80+
}
81+
UNLOCK_WEAKREFS(obj);
82+
return NULL;
5383
}
5484

5585
static inline int _PyWeakref_IS_DEAD(PyObject *ref_obj)
5686
{
5787
assert(PyWeakref_Check(ref_obj));
5888
int ret = 0;
59-
Py_BEGIN_CRITICAL_SECTION(ref_obj);
6089
PyWeakReference *ref = _Py_CAST(PyWeakReference*, ref_obj);
61-
PyObject *obj = ref->wr_object;
90+
PyObject *obj = FT_ATOMIC_LOAD_PTR(ref->wr_object);
6291
if (obj == Py_None) {
6392
// clear_weakref() was called
6493
ret = 1;
6594
}
6695
else {
96+
LOCK_WEAKREFS(obj);
6797
// See _PyWeakref_GET_REF() for the rationale of this test
98+
#ifdef Py_GIL_DISABLED
99+
ret = (ref->wr_object == Py_None) || _is_dead(obj);
100+
#else
68101
ret = _is_dead(obj);
102+
#endif
103+
UNLOCK_WEAKREFS(obj);
69104
}
70-
Py_END_CRITICAL_SECTION();
71105
return ret;
72106
}
73107

74-
extern Py_ssize_t _PyWeakref_GetWeakrefCount(PyWeakReference *head);
108+
extern Py_ssize_t _PyWeakref_GetWeakrefCount(PyObject *obj);
109+
110+
// Clear all the weak references to obj but leave their callbacks uncalled and
111+
// intact.
112+
extern void _PyWeakref_ClearWeakRefsExceptCallbacks(PyObject *obj);
75113

76114
extern void _PyWeakref_ClearRef(PyWeakReference *self);
77115

116+
PyAPI_FUNC(int) _PyWeakref_IsDead(PyObject *weakref);
117+
78118
#ifdef __cplusplus
79119
}
80120
#endif
81121
#endif /* !Py_INTERNAL_WEAKREF_H */
82-

‎Lib/test/test_sys.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1708,11 +1708,15 @@ class newstyleclass(object): pass
17081708
# TODO: add check that forces layout of unicodefields
17091709
# weakref
17101710
import weakref
1711-
check(weakref.ref(int), size('2Pn3P'))
1711+
if support.Py_GIL_DISABLED:
1712+
expected = size('2Pn4P')
1713+
else:
1714+
expected = size('2Pn3P')
1715+
check(weakref.ref(int), expected)
17121716
# weakproxy
17131717
# XXX
17141718
# weakcallableproxy
1715-
check(weakref.proxy(int), size('2Pn3P'))
1719+
check(weakref.proxy(int), expected)
17161720

17171721
def check_slots(self, obj, base, extra):
17181722
expected = sys.getsizeof(base) + struct.calcsize(extra)

‎Lib/test/test_weakref.py‎

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

1910+
@support.cpython_only
1911+
def test_weak_valued_consistency(self):
1912+
# A single-threaded, deterministic repro for issue #28427: old keys
1913+
# should not remove new values from WeakValueDictionary. This relies on
1914+
# an implementation detail of CPython's WeakValueDictionary (its
1915+
# underlying dictionary of KeyedRefs) to reproduce the issue.
1916+
d = weakref.WeakValueDictionary()
1917+
with support.disable_gc():
1918+
d[10] = RefCycle()
1919+
# Keep the KeyedRef alive after it's replaced so that GC will invoke
1920+
# the callback.
1921+
wr = d.data[10]
1922+
# Replace the value with something that isn't cyclic garbage
1923+
o = RefCycle()
1924+
d[10] = o
1925+
# Trigger GC, which will invoke the callback for `wr`
1926+
gc.collect()
1927+
self.assertEqual(len(d), 1)
1928+
19101929
def check_threaded_weak_dict_copy(self, type_, deepcopy):
19111930
# `type_` should be either WeakKeyDictionary or WeakValueDictionary.
19121931
# `deepcopy` should be either True or False.

‎Modules/_sqlite/blob.c‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
#include "blob.h"
66
#include "util.h"
7-
#include "pycore_weakref.h" // _PyWeakref_GET_REF()
87

98
#define clinic_state() (pysqlite_get_state_by_type(Py_TYPE(self)))
109
#include "clinic/blob.c.h"
@@ -102,8 +101,8 @@ pysqlite_close_all_blobs(pysqlite_Connection *self)
102101
{
103102
for (int i = 0; i < PyList_GET_SIZE(self->blobs); i++) {
104103
PyObject *weakref = PyList_GET_ITEM(self->blobs, i);
105-
PyObject *blob = _PyWeakref_GET_REF(weakref);
106-
if (blob == NULL) {
104+
PyObject *blob;
105+
if (!PyWeakref_GetRef(weakref, &blob)) {
107106
continue;
108107
}
109108
close_blob((pysqlite_Blob *)blob);

‎Modules/_sqlite/connection.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
#include "pycore_modsupport.h" // _PyArg_NoKeywords()
3939
#include "pycore_pyerrors.h" // _PyErr_ChainExceptions1()
4040
#include "pycore_pylifecycle.h" // _Py_IsInterpreterFinalizing()
41-
#include "pycore_weakref.h" // _PyWeakref_IS_DEAD()
41+
#include "pycore_weakref.h"
4242

4343
#include <stdbool.h>
4444

@@ -1065,7 +1065,7 @@ static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self)
10651065

10661066
for (Py_ssize_t i = 0; i < PyList_Size(self->cursors); i++) {
10671067
PyObject* weakref = PyList_GetItem(self->cursors, i);
1068-
if (_PyWeakref_IS_DEAD(weakref)) {
1068+
if (_PyWeakref_IsDead(weakref)) {
10691069
continue;
10701070
}
10711071
if (PyList_Append(new_list, weakref) != 0) {

‎Modules/_ssl.c‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
#include "pycore_fileutils.h" // _PyIsSelectable_fd()
3030
#include "pycore_pyerrors.h" // _PyErr_ChainExceptions1()
3131
#include "pycore_time.h" // _PyDeadline_Init()
32-
#include "pycore_weakref.h" // _PyWeakref_GET_REF()
3332

3433
/* Include symbols from _socket module */
3534
#include "socketmodule.h"
@@ -392,8 +391,8 @@ typedef enum {
392391
// Return a borrowed reference.
393392
static inline PySocketSockObject* GET_SOCKET(PySSLSocket *obj) {
394393
if (obj->Socket) {
395-
PyObject *sock = _PyWeakref_GET_REF(obj->Socket);
396-
if (sock != NULL) {
394+
PyObject *sock;
395+
if (PyWeakref_GetRef(obj->Socket, &sock)) {
397396
// GET_SOCKET() returns a borrowed reference
398397
Py_DECREF(sock);
399398
}
@@ -2205,8 +2204,8 @@ PySSL_get_owner(PySSLSocket *self, void *c)
22052204
if (self->owner == NULL) {
22062205
Py_RETURN_NONE;
22072206
}
2208-
PyObject *owner = _PyWeakref_GET_REF(self->owner);
2209-
if (owner == NULL) {
2207+
PyObject *owner;
2208+
if (!PyWeakref_GetRef(self->owner, &owner)) {
22102209
Py_RETURN_NONE;
22112210
}
22122211
return owner;
@@ -4433,9 +4432,9 @@ _servername_callback(SSL *s, int *al, void *args)
44334432
* will be passed. If both do not exist only then the C-level object is
44344433
* passed. */
44354434
if (ssl->owner)
4436-
ssl_socket = _PyWeakref_GET_REF(ssl->owner);
4435+
PyWeakref_GetRef(ssl->owner, &ssl_socket);
44374436
else if (ssl->Socket)
4438-
ssl_socket = _PyWeakref_GET_REF(ssl->Socket);
4437+
PyWeakref_GetRef(ssl->Socket, &ssl_socket);
44394438
else
44404439
ssl_socket = Py_NewRef(ssl);
44414440

0 commit comments

Comments
 (0)
pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.




Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy