Skip to content

Commit 94444ea

Browse files
corona10corona10
authored
gh-112069: Add _PySet_NextEntryRef to be thread-safe. (gh-117990)
1 parent 40f4d64 commit 94444ea

File tree

9 files changed

+76
-34
lines changed

9 files changed

+76
-34
lines changed

‎Include/internal/pycore_setobject.h

+8-1
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,20 @@ extern "C" {
88
# error "this header requires Py_BUILD_CORE define"
99
#endif
1010

11-
// Export for '_pickle' shared extension
11+
// Export for '_abc' shared extension
1212
PyAPI_FUNC(int) _PySet_NextEntry(
1313
PyObject *set,
1414
Py_ssize_t *pos,
1515
PyObject **key,
1616
Py_hash_t *hash);
1717

18+
// Export for '_pickle' shared extension
19+
PyAPI_FUNC(int) _PySet_NextEntryRef(
20+
PyObject *set,
21+
Py_ssize_t *pos,
22+
PyObject **key,
23+
Py_hash_t *hash);
24+
1825
// Export for '_pickle' shared extension
1926
PyAPI_FUNC(int) _PySet_Update(PyObject *set, PyObject *iterable);
2027

‎Modules/_abc.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,7 @@ subclasscheck_check_registry(_abc_data *impl, PyObject *subclass,
862862

863863
// Make a local copy of the registry to protect against concurrent
864864
// modifications of _abc_registry.
865-
PyObject *registry = PySet_New(registry_shared);
865+
PyObject *registry = PyFrozenSet_New(registry_shared);
866866
if (registry == NULL) {
867867
return -1;
868868
}

‎Modules/_pickle.c

+20-13
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,16 @@
99
#endif
1010

1111
#include "Python.h"
12-
#include "pycore_bytesobject.h" // _PyBytesWriter
13-
#include "pycore_ceval.h" // _Py_EnterRecursiveCall()
14-
#include "pycore_long.h" // _PyLong_AsByteArray()
15-
#include "pycore_moduleobject.h" // _PyModule_GetState()
16-
#include "pycore_object.h" // _PyNone_Type
17-
#include "pycore_pystate.h" // _PyThreadState_GET()
18-
#include "pycore_runtime.h" // _Py_ID()
19-
#include "pycore_setobject.h" // _PySet_NextEntry()
20-
#include "pycore_sysmodule.h" // _PySys_GetAttr()
12+
#include "pycore_bytesobject.h" // _PyBytesWriter
13+
#include "pycore_ceval.h" // _Py_EnterRecursiveCall()
14+
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION()
15+
#include "pycore_long.h" // _PyLong_AsByteArray()
16+
#include "pycore_moduleobject.h" // _PyModule_GetState()
17+
#include "pycore_object.h" // _PyNone_Type
18+
#include "pycore_pystate.h" // _PyThreadState_GET()
19+
#include "pycore_runtime.h" // _Py_ID()
20+
#include "pycore_setobject.h" // _PySet_NextEntry()
21+
#include "pycore_sysmodule.h" // _PySys_GetAttr()
2122

2223
#include <stdlib.h> // strtol()
2324

@@ -3413,15 +3414,21 @@ save_set(PickleState *state, PicklerObject *self, PyObject *obj)
34133414
i = 0;
34143415
if (_Pickler_Write(self, &mark_op, 1) < 0)
34153416
return -1;
3416-
while (_PySet_NextEntry(obj, &ppos, &item, &hash)) {
3417-
Py_INCREF(item);
3418-
int err = save(state, self, item, 0);
3417+
3418+
int err = 0;
3419+
Py_BEGIN_CRITICAL_SECTION(obj);
3420+
while (_PySet_NextEntryRef(obj, &ppos, &item, &hash)) {
3421+
err = save(state, self, item, 0);
34193422
Py_CLEAR(item);
34203423
if (err < 0)
3421-
return -1;
3424+
break;
34223425
if (++i == BATCHSIZE)
34233426
break;
34243427
}
3428+
Py_END_CRITICAL_SECTION();
3429+
if (err < 0) {
3430+
return -1;
3431+
}
34253432
if (_Pickler_Write(self, &additems_op, 1) < 0)
34263433
return -1;
34273434
if (PySet_GET_SIZE(obj) != set_size) {

‎Modules/_testinternalcapi/set.c

+7-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "parts.h"
22
#include "../_testcapi/util.h" // NULLABLE, RETURN_INT
33

4+
#include "pycore_critical_section.h"
45
#include "pycore_setobject.h"
56

67

@@ -27,10 +28,13 @@ set_next_entry(PyObject *self, PyObject *args)
2728
return NULL;
2829
}
2930
NULLABLE(set);
30-
31-
rc = _PySet_NextEntry(set, &pos, &item, &hash);
31+
Py_BEGIN_CRITICAL_SECTION(set);
32+
rc = _PySet_NextEntryRef(set, &pos, &item, &hash);
33+
Py_END_CRITICAL_SECTION();
3234
if (rc == 1) {
33-
return Py_BuildValue("innO", rc, pos, hash, item);
35+
PyObject *ret = Py_BuildValue("innO", rc, pos, hash, item);
36+
Py_DECREF(item);
37+
return ret;
3438
}
3539
assert(item == UNINITIALIZED_PTR);
3640
assert(hash == (Py_hash_t)UNINITIALIZED_SIZE);

‎Objects/dictobject.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -2979,8 +2979,9 @@ dict_set_fromkeys(PyInterpreterState *interp, PyDictObject *mp,
29792979
return NULL;
29802980
}
29812981

2982-
while (_PySet_NextEntry(iterable, &pos, &key, &hash)) {
2983-
if (insertdict(interp, mp, Py_NewRef(key), hash, Py_NewRef(value))) {
2982+
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(iterable);
2983+
while (_PySet_NextEntryRef(iterable, &pos, &key, &hash)) {
2984+
if (insertdict(interp, mp, key, hash, Py_NewRef(value))) {
29842985
Py_DECREF(mp);
29852986
return NULL;
29862987
}

‎Objects/listobject.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -1287,8 +1287,7 @@ list_extend_set(PyListObject *self, PySetObject *other)
12871287
Py_hash_t hash;
12881288
PyObject *key;
12891289
PyObject **dest = self->ob_item + m;
1290-
while (_PySet_NextEntry((PyObject *)other, &setpos, &key, &hash)) {
1291-
Py_INCREF(key);
1290+
while (_PySet_NextEntryRef((PyObject *)other, &setpos, &key, &hash)) {
12921291
FT_ATOMIC_STORE_PTR_RELEASE(*dest, key);
12931292
dest++;
12941293
}

‎Objects/setobject.c

+17-1
Original file line numberDiff line numberDiff line change
@@ -2661,7 +2661,6 @@ PySet_Add(PyObject *anyset, PyObject *key)
26612661
return rv;
26622662
}
26632663

2664-
// TODO: Make thread-safe in free-threaded builds
26652664
int
26662665
_PySet_NextEntry(PyObject *set, Py_ssize_t *pos, PyObject **key, Py_hash_t *hash)
26672666
{
@@ -2678,6 +2677,23 @@ _PySet_NextEntry(PyObject *set, Py_ssize_t *pos, PyObject **key, Py_hash_t *hash
26782677
return 1;
26792678
}
26802679

2680+
int
2681+
_PySet_NextEntryRef(PyObject *set, Py_ssize_t *pos, PyObject **key, Py_hash_t *hash)
2682+
{
2683+
setentry *entry;
2684+
2685+
if (!PyAnySet_Check(set)) {
2686+
PyErr_BadInternalCall();
2687+
return -1;
2688+
}
2689+
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(set);
2690+
if (set_next((PySetObject *)set, pos, &entry) == 0)
2691+
return 0;
2692+
*key = Py_NewRef(entry->key);
2693+
*hash = entry->hash;
2694+
return 1;
2695+
}
2696+
26812697
PyObject *
26822698
PySet_Pop(PyObject *set)
26832699
{

‎Python/marshal.c

+18-11
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@
77
and sharing. */
88

99
#include "Python.h"
10-
#include "pycore_call.h" // _PyObject_CallNoArgs()
11-
#include "pycore_code.h" // _PyCode_New()
12-
#include "pycore_hashtable.h" // _Py_hashtable_t
13-
#include "pycore_long.h" // _PyLong_DigitCount
14-
#include "pycore_setobject.h" // _PySet_NextEntry()
15-
#include "marshal.h" // Py_MARSHAL_VERSION
10+
#include "pycore_call.h" // _PyObject_CallNoArgs()
11+
#include "pycore_code.h" // _PyCode_New()
12+
#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION()
13+
#include "pycore_hashtable.h" // _Py_hashtable_t
14+
#include "pycore_long.h" // _PyLong_DigitCount
15+
#include "pycore_setobject.h" // _PySet_NextEntry()
16+
#include "marshal.h" // Py_MARSHAL_VERSION
1617

1718
#ifdef __APPLE__
1819
# include "TargetConditionals.h"
@@ -531,23 +532,29 @@ w_complex_object(PyObject *v, char flag, WFILE *p)
531532
return;
532533
}
533534
Py_ssize_t i = 0;
534-
while (_PySet_NextEntry(v, &pos, &value, &hash)) {
535+
Py_BEGIN_CRITICAL_SECTION(v);
536+
while (_PySet_NextEntryRef(v, &pos, &value, &hash)) {
535537
PyObject *dump = _PyMarshal_WriteObjectToString(value,
536538
p->version, p->allow_code);
537539
if (dump == NULL) {
538540
p->error = WFERR_UNMARSHALLABLE;
539-
Py_DECREF(pairs);
540-
return;
541+
Py_DECREF(value);
542+
break;
541543
}
542544
PyObject *pair = PyTuple_Pack(2, dump, value);
543545
Py_DECREF(dump);
546+
Py_DECREF(value);
544547
if (pair == NULL) {
545548
p->error = WFERR_NOMEMORY;
546-
Py_DECREF(pairs);
547-
return;
549+
break;
548550
}
549551
PyList_SET_ITEM(pairs, i++, pair);
550552
}
553+
Py_END_CRITICAL_SECTION();
554+
if (p->error == WFERR_UNMARSHALLABLE || p->error == WFERR_NOMEMORY) {
555+
Py_DECREF(pairs);
556+
return;
557+
}
551558
assert(i == n);
552559
if (PyList_Sort(pairs)) {
553560
p->error = WFERR_NOMEMORY;

‎Python/pylifecycle.c

+1
Original file line numberDiff line numberDiff line change
@@ -2910,6 +2910,7 @@ _Py_DumpExtensionModules(int fd, PyInterpreterState *interp)
29102910
Py_ssize_t i = 0;
29112911
PyObject *item;
29122912
Py_hash_t hash;
2913+
// if stdlib_module_names is not NULL, it is always a frozenset.
29132914
while (_PySet_NextEntry(stdlib_module_names, &i, &item, &hash)) {
29142915
if (PyUnicode_Check(item)
29152916
&& PyUnicode_Compare(key, item) == 0)

0 commit comments

Comments
 (0)