From 131cb800420ebf818fa00f42851b75e479629bd9 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 11 Feb 2024 15:36:39 +0100 Subject: [PATCH 01/26] Use `@critical_section` where possible --- Objects/clinic/setobject.c.h | 186 +++++++++++++++++++++++++++++++++-- Objects/setobject.c | 63 +++++++----- 2 files changed, 218 insertions(+), 31 deletions(-) diff --git a/Objects/clinic/setobject.c.h b/Objects/clinic/setobject.c.h index f3c96995ede60d..a0b7bd7084dc29 100644 --- a/Objects/clinic/setobject.c.h +++ b/Objects/clinic/setobject.c.h @@ -2,6 +2,7 @@ preserve [clinic start generated code]*/ +#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() #include "pycore_modsupport.h" // _PyArg_CheckPositional() PyDoc_STRVAR(set_pop__doc__, @@ -21,7 +22,13 @@ set_pop_impl(PySetObject *so); static PyObject * set_pop(PySetObject *so, PyObject *Py_UNUSED(ignored)) { - return set_pop_impl(so); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(so); + return_value = set_pop_impl(so); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(set_update__doc__, @@ -74,7 +81,13 @@ set_copy_impl(PySetObject *so); static PyObject * set_copy(PySetObject *so, PyObject *Py_UNUSED(ignored)) { - return set_copy_impl(so); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(so); + return_value = set_copy_impl(so); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(frozenset_copy__doc__, @@ -92,7 +105,13 @@ frozenset_copy_impl(PySetObject *so); static PyObject * frozenset_copy(PySetObject *so, PyObject *Py_UNUSED(ignored)) { - return frozenset_copy_impl(so); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(so); + return_value = frozenset_copy_impl(so); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(set_clear__doc__, @@ -110,7 +129,13 @@ set_clear_impl(PySetObject *so); static PyObject * set_clear(PySetObject *so, PyObject *Py_UNUSED(ignored)) { - return set_clear_impl(so); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(so); + return_value = set_clear_impl(so); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(set_union__doc__, @@ -227,6 +252,21 @@ PyDoc_STRVAR(set_isdisjoint__doc__, #define SET_ISDISJOINT_METHODDEF \ {"isdisjoint", (PyCFunction)set_isdisjoint, METH_O, set_isdisjoint__doc__}, +static PyObject * +set_isdisjoint_impl(PySetObject *so, PyObject *other); + +static PyObject * +set_isdisjoint(PySetObject *so, PyObject *other) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION2(so, other); + return_value = set_isdisjoint_impl(so, other); + Py_END_CRITICAL_SECTION2(); + + return return_value; +} + PyDoc_STRVAR(set_difference_update__doc__, "difference_update($self, /, *others)\n" "--\n" @@ -306,6 +346,21 @@ PyDoc_STRVAR(set_symmetric_difference_update__doc__, #define SET_SYMMETRIC_DIFFERENCE_UPDATE_METHODDEF \ {"symmetric_difference_update", (PyCFunction)set_symmetric_difference_update, METH_O, set_symmetric_difference_update__doc__}, +static PyObject * +set_symmetric_difference_update_impl(PySetObject *so, PyObject *other); + +static PyObject * +set_symmetric_difference_update(PySetObject *so, PyObject *other) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION2(so, other); + return_value = set_symmetric_difference_update_impl(so, other); + Py_END_CRITICAL_SECTION2(); + + return return_value; +} + PyDoc_STRVAR(set_symmetric_difference__doc__, "symmetric_difference($self, other, /)\n" "--\n" @@ -315,6 +370,21 @@ PyDoc_STRVAR(set_symmetric_difference__doc__, #define SET_SYMMETRIC_DIFFERENCE_METHODDEF \ {"symmetric_difference", (PyCFunction)set_symmetric_difference, METH_O, set_symmetric_difference__doc__}, +static PyObject * +set_symmetric_difference_impl(PySetObject *so, PyObject *other); + +static PyObject * +set_symmetric_difference(PySetObject *so, PyObject *other) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION2(so, other); + return_value = set_symmetric_difference_impl(so, other); + Py_END_CRITICAL_SECTION2(); + + return return_value; +} + PyDoc_STRVAR(set_issubset__doc__, "issubset($self, other, /)\n" "--\n" @@ -324,6 +394,21 @@ PyDoc_STRVAR(set_issubset__doc__, #define SET_ISSUBSET_METHODDEF \ {"issubset", (PyCFunction)set_issubset, METH_O, set_issubset__doc__}, +static PyObject * +set_issubset_impl(PySetObject *so, PyObject *other); + +static PyObject * +set_issubset(PySetObject *so, PyObject *other) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION2(so, other); + return_value = set_issubset_impl(so, other); + Py_END_CRITICAL_SECTION2(); + + return return_value; +} + PyDoc_STRVAR(set_issuperset__doc__, "issuperset($self, other, /)\n" "--\n" @@ -333,6 +418,21 @@ PyDoc_STRVAR(set_issuperset__doc__, #define SET_ISSUPERSET_METHODDEF \ {"issuperset", (PyCFunction)set_issuperset, METH_O, set_issuperset__doc__}, +static PyObject * +set_issuperset_impl(PySetObject *so, PyObject *other); + +static PyObject * +set_issuperset(PySetObject *so, PyObject *other) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION2(so, other); + return_value = set_issuperset_impl(so, other); + Py_END_CRITICAL_SECTION2(); + + return return_value; +} + PyDoc_STRVAR(set_add__doc__, "add($self, object, /)\n" "--\n" @@ -344,6 +444,21 @@ PyDoc_STRVAR(set_add__doc__, #define SET_ADD_METHODDEF \ {"add", (PyCFunction)set_add, METH_O, set_add__doc__}, +static PyObject * +set_add_impl(PySetObject *so, PyObject *key); + +static PyObject * +set_add(PySetObject *so, PyObject *key) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(so); + return_value = set_add_impl(so, key); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(set___contains____doc__, "__contains__($self, object, /)\n" "--\n" @@ -353,6 +468,21 @@ PyDoc_STRVAR(set___contains____doc__, #define SET___CONTAINS___METHODDEF \ {"__contains__", (PyCFunction)set___contains__, METH_O|METH_COEXIST, set___contains____doc__}, +static PyObject * +set___contains___impl(PySetObject *so, PyObject *key); + +static PyObject * +set___contains__(PySetObject *so, PyObject *key) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(so); + return_value = set___contains___impl(so, key); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(set_remove__doc__, "remove($self, object, /)\n" "--\n" @@ -364,6 +494,21 @@ PyDoc_STRVAR(set_remove__doc__, #define SET_REMOVE_METHODDEF \ {"remove", (PyCFunction)set_remove, METH_O, set_remove__doc__}, +static PyObject * +set_remove_impl(PySetObject *so, PyObject *key); + +static PyObject * +set_remove(PySetObject *so, PyObject *key) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(so); + return_value = set_remove_impl(so, key); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(set_discard__doc__, "discard($self, object, /)\n" "--\n" @@ -376,6 +521,21 @@ PyDoc_STRVAR(set_discard__doc__, #define SET_DISCARD_METHODDEF \ {"discard", (PyCFunction)set_discard, METH_O, set_discard__doc__}, +static PyObject * +set_discard_impl(PySetObject *so, PyObject *key); + +static PyObject * +set_discard(PySetObject *so, PyObject *key) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(so); + return_value = set_discard_impl(so, key); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + PyDoc_STRVAR(set___reduce____doc__, "__reduce__($self, /)\n" "--\n" @@ -391,7 +551,13 @@ set___reduce___impl(PySetObject *so); static PyObject * set___reduce__(PySetObject *so, PyObject *Py_UNUSED(ignored)) { - return set___reduce___impl(so); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(so); + return_value = set___reduce___impl(so); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(set___sizeof____doc__, @@ -409,6 +575,12 @@ set___sizeof___impl(PySetObject *so); static PyObject * set___sizeof__(PySetObject *so, PyObject *Py_UNUSED(ignored)) { - return set___sizeof___impl(so); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(so); + return_value = set___sizeof___impl(so); + Py_END_CRITICAL_SECTION(); + + return return_value; } -/*[clinic end generated code: output=34a30591148da884 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=e320fd760fa48265 input=a9049054013a1b77]*/ diff --git a/Objects/setobject.c b/Objects/setobject.c index 6a4c8c45f0836d..9288785164d3dc 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -645,6 +645,7 @@ set_merge(PySetObject *so, PyObject *otherset) } /*[clinic input] +@critical_section set.pop so: setobject @@ -655,7 +656,7 @@ Raises KeyError if the set is empty. static PyObject * set_pop_impl(PySetObject *so) -/*[clinic end generated code: output=4d65180f1271871b input=4a3f5552e660a260]*/ +/*[clinic end generated code: output=4d65180f1271871b input=9296c84921125060]*/ { /* Make sure the search finger is in bounds */ setentry *entry = so->table + (so->finger & so->mask); @@ -1126,6 +1127,7 @@ set_swap_bodies(PySetObject *a, PySetObject *b) } /*[clinic input] +@critical_section set.copy so: setobject @@ -1134,12 +1136,13 @@ Return a shallow copy of a set. static PyObject * set_copy_impl(PySetObject *so) -/*[clinic end generated code: output=c9223a1e1cc6b041 input=2b80b288d47b8cf1]*/ +/*[clinic end generated code: output=c9223a1e1cc6b041 input=c169a4fbb8209257]*/ { return make_new_set_basetype(Py_TYPE(so), (PyObject *)so); } /*[clinic input] +@critical_section frozenset.copy so: setobject @@ -1148,7 +1151,7 @@ Return a shallow copy of a set. static PyObject * frozenset_copy_impl(PySetObject *so) -/*[clinic end generated code: output=b356263526af9e70 input=3dc65577d344eff7]*/ +/*[clinic end generated code: output=b356263526af9e70 input=fbf5bef131268dd7]*/ { if (PyFrozenSet_CheckExact(so)) { return Py_NewRef(so); @@ -1157,6 +1160,7 @@ frozenset_copy_impl(PySetObject *so) } /*[clinic input] +@critical_section set.clear so: setobject @@ -1165,7 +1169,7 @@ Remove all elements from this set. static PyObject * set_clear_impl(PySetObject *so) -/*[clinic end generated code: output=4e71d5a83904161a input=74ac19794da81a39]*/ +/*[clinic end generated code: output=4e71d5a83904161a input=c6f831b366111950]*/ { set_clear_internal(so); Py_RETURN_NONE; @@ -1408,6 +1412,7 @@ set_iand(PySetObject *so, PyObject *other) } /*[clinic input] +@critical_section so other set.isdisjoint so: setobject other: object @@ -1417,8 +1422,8 @@ Return True if two sets have a null intersection. [clinic start generated code]*/ static PyObject * -set_isdisjoint(PySetObject *so, PyObject *other) -/*[clinic end generated code: output=a92bbf9a2db6a3da input=c254ddec8a2326e3]*/ +set_isdisjoint_impl(PySetObject *so, PyObject *other) +/*[clinic end generated code: output=273493f2d57c565e input=32f8dcab5e0fc7d6]*/ { PyObject *key, *it, *tmp; int rv; @@ -1727,6 +1732,7 @@ set_symmetric_difference_update_dict(PySetObject *so, PyObject *other) } /*[clinic input] +@critical_section so other set.symmetric_difference_update so: setobject other: object @@ -1736,8 +1742,8 @@ Update the set, keeping only elements found in either set, but not in both. [clinic start generated code]*/ static PyObject * -set_symmetric_difference_update(PySetObject *so, PyObject *other) -/*[clinic end generated code: output=fbb049c0806028de input=a50acf0365e1f0a5]*/ +set_symmetric_difference_update_impl(PySetObject *so, PyObject *other) +/*[clinic end generated code: output=79f80b4ee5da66c1 input=34c143ee8c2ba08b]*/ { PySetObject *otherset; PyObject *key; @@ -1791,6 +1797,7 @@ set_symmetric_difference_update(PySetObject *so, PyObject *other) } /*[clinic input] +@critical_section so other set.symmetric_difference so: setobject other: object @@ -1800,8 +1807,8 @@ Return a new set with elements in either the set or other but not both. [clinic start generated code]*/ static PyObject * -set_symmetric_difference(PySetObject *so, PyObject *other) -/*[clinic end generated code: output=f95364211b88775a input=f18af370ad72ebac]*/ +set_symmetric_difference_impl(PySetObject *so, PyObject *other) +/*[clinic end generated code: output=270ee0b5d42b0797 input=624f6e7bbdf70db1]*/ { PyObject *rv; PySetObject *otherset; @@ -1841,6 +1848,7 @@ set_ixor(PySetObject *so, PyObject *other) } /*[clinic input] +@critical_section so other set.issubset so: setobject other: object @@ -1850,8 +1858,8 @@ Report whether another set contains this set. [clinic start generated code]*/ static PyObject * -set_issubset(PySetObject *so, PyObject *other) -/*[clinic end generated code: output=78aef1f377aedef1 input=37fbc579b609db0c]*/ +set_issubset_impl(PySetObject *so, PyObject *other) +/*[clinic end generated code: output=b2b59d5f314555ce input=f2a4fd0f2537758b]*/ { setentry *entry; Py_ssize_t pos = 0; @@ -1885,6 +1893,7 @@ set_issubset(PySetObject *so, PyObject *other) } /*[clinic input] +@critical_section so other set.issuperset so: setobject other: object @@ -1894,8 +1903,8 @@ Report whether this set contains another set. [clinic start generated code]*/ static PyObject * -set_issuperset(PySetObject *so, PyObject *other) -/*[clinic end generated code: output=7d2b71dd714a7ec7 input=fd5dab052f2e9bb3]*/ +set_issuperset_impl(PySetObject *so, PyObject *other) +/*[clinic end generated code: output=ecf00ce552c09461 input=5f2e1f262e6e4ccc]*/ { if (PyAnySet_Check(other)) { return set_issubset((PySetObject *)other, (PyObject *)so); @@ -1968,6 +1977,7 @@ set_richcompare(PySetObject *v, PyObject *w, int op) } /*[clinic input] +@critical_section set.add so: setobject object as key: object @@ -1979,8 +1989,8 @@ This has no effect if the element is already present. [clinic start generated code]*/ static PyObject * -set_add(PySetObject *so, PyObject *key) -/*[clinic end generated code: output=cd9c2d5c2069c2ba input=96f1efe029e47972]*/ +set_add_impl(PySetObject *so, PyObject *key) +/*[clinic end generated code: output=4cc4a937f1425c96 input=03baf62cb0e66514]*/ { if (set_add_key(so, key)) return NULL; @@ -2008,6 +2018,7 @@ set_contains(PySetObject *so, PyObject *key) } /*[clinic input] +@critical_section @coexist set.__contains__ so: setobject @@ -2018,8 +2029,8 @@ x.__contains__(y) <==> y in x. [clinic start generated code]*/ static PyObject * -set___contains__(PySetObject *so, PyObject *key) -/*[clinic end generated code: output=b5948bc5c590d3ca input=cf4c72db704e4cf0]*/ +set___contains___impl(PySetObject *so, PyObject *key) +/*[clinic end generated code: output=b44863d034b3c70e input=4a7d568459617f24]*/ { long result; @@ -2030,6 +2041,7 @@ set___contains__(PySetObject *so, PyObject *key) } /*[clinic input] +@critical_section set.remove so: setobject object as key: object @@ -2041,8 +2053,8 @@ If the element is not a member, raise a KeyError. [clinic start generated code]*/ static PyObject * -set_remove(PySetObject *so, PyObject *key) -/*[clinic end generated code: output=08ae496d0cd2b8c1 input=10132515dfe8ebd7]*/ +set_remove_impl(PySetObject *so, PyObject *key) +/*[clinic end generated code: output=0b9134a2a2200363 input=893e1cb1df98227a]*/ { PyObject *tmpkey; int rv; @@ -2069,6 +2081,7 @@ set_remove(PySetObject *so, PyObject *key) } /*[clinic input] +@critical_section set.discard so: setobject object as key: object @@ -2081,8 +2094,8 @@ an exception when an element is missing from the set. [clinic start generated code]*/ static PyObject * -set_discard(PySetObject *so, PyObject *key) -/*[clinic end generated code: output=9181b60d7bb7d480 input=82a689eba94d5ad9]*/ +set_discard_impl(PySetObject *so, PyObject *key) +/*[clinic end generated code: output=eec3b687bf32759e input=861cb7fb69b4def0]*/ { PyObject *tmpkey; int rv; @@ -2104,6 +2117,7 @@ set_discard(PySetObject *so, PyObject *key) } /*[clinic input] +@critical_section set.__reduce__ so: setobject @@ -2112,7 +2126,7 @@ Return state information for pickling. static PyObject * set___reduce___impl(PySetObject *so) -/*[clinic end generated code: output=9af7d0e029df87ee input=531375e87a24a449]*/ +/*[clinic end generated code: output=9af7d0e029df87ee input=59405a4249e82f71]*/ { PyObject *keys=NULL, *args=NULL, *result=NULL, *state=NULL; @@ -2134,6 +2148,7 @@ set___reduce___impl(PySetObject *so) } /*[clinic input] +@critical_section set.__sizeof__ so: setobject @@ -2142,7 +2157,7 @@ S.__sizeof__() -> size of S in memory, in bytes. static PyObject * set___sizeof___impl(PySetObject *so) -/*[clinic end generated code: output=4bfa3df7bd38ed88 input=0f214fc2225319fc]*/ +/*[clinic end generated code: output=4bfa3df7bd38ed88 input=09e1a09f168eaa23]*/ { size_t res = _PyObject_SIZE(Py_TYPE(so)); if (so->table != so->smalltable) { From 7d2925a96117e9135a63306993d62e222ea307d5 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 11 Feb 2024 15:46:30 +0100 Subject: [PATCH 02/26] Convert `set_len` --- Objects/setobject.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 9288785164d3dc..89dfffe9028a25 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -568,9 +568,9 @@ set_repr(PySetObject *so) } static Py_ssize_t -set_len(PyObject *so) +set_len(PySetObject *so) { - return ((PySetObject *)so)->used; + return _Py_atomic_load_ssize_relaxed(&so->used); } static int @@ -2206,7 +2206,7 @@ set_vectorcall(PyObject *type, PyObject * const*args, } static PySequenceMethods set_as_sequence = { - set_len, /* sq_length */ + (lenfunc)set_len, /* sq_length */ 0, /* sq_concat */ 0, /* sq_repeat */ 0, /* sq_item */ From 4410da9b85b36ecbc34791707cc705c872556299 Mon Sep 17 00:00:00 2001 From: Tomas R Date: Fri, 12 Jan 2024 10:37:44 +0100 Subject: [PATCH 03/26] Add TODO comments --- Objects/setobject.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Objects/setobject.c b/Objects/setobject.c index 89dfffe9028a25..de8f507642f597 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1933,6 +1933,7 @@ set_issuperset_impl(PySetObject *so, PyObject *other) Py_RETURN_TRUE; } +// TODO: Make thread-safe in free-threaded builds static PyObject * set_richcompare(PySetObject *v, PyObject *w, int op) { @@ -2483,6 +2484,7 @@ PySet_Add(PyObject *anyset, PyObject *key) return set_add_key((PySetObject *)anyset, key); } +// TODO: Make thread-safe in free-threaded builds int _PySet_NextEntry(PyObject *set, Py_ssize_t *pos, PyObject **key, Py_hash_t *hash) { From 87c6c68bd8bfa9c656762bb5cebaf2bd11fb2d89 Mon Sep 17 00:00:00 2001 From: Tomas R Date: Fri, 12 Jan 2024 10:38:18 +0100 Subject: [PATCH 04/26] Make `set_iter` thread-safe --- Objects/setobject.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index de8f507642f597..224db46a28ee0b 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -890,13 +890,14 @@ PyTypeObject PySetIter_Type = { static PyObject * set_iter(PySetObject *so) { + Py_ssize_t size = set_len(so); setiterobject *si = PyObject_GC_New(setiterobject, &PySetIter_Type); if (si == NULL) return NULL; si->si_set = (PySetObject*)Py_NewRef(so); - si->si_used = so->used; + si->si_used = size; si->si_pos = 0; - si->len = so->used; + si->len = size; _PyObject_GC_TRACK(si); return (PyObject *)si; } From 497d0622ddac52c70e835a96df98fcc9f3d4d1f2 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 11 Feb 2024 16:01:00 +0100 Subject: [PATCH 05/26] Convert all C API functions --- Objects/setobject.c | 57 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 224db46a28ee0b..f86f7f1344794e 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2425,13 +2425,23 @@ PyTypeObject PyFrozenSet_Type = { PyObject * PySet_New(PyObject *iterable) { - return make_new_set(&PySet_Type, iterable); + PyObject *rv; + + Py_BEGIN_CRITICAL_SECTION(iterable); + rv = make_new_set(&PySet_Type, iterable); + Py_END_CRITICAL_SECTION(); + return rv; } PyObject * PyFrozenSet_New(PyObject *iterable) { - return make_new_set(&PyFrozenSet_Type, iterable); + PyObject *rv; + + Py_BEGIN_CRITICAL_SECTION(iterable); + rv = make_new_set(&PyFrozenSet_Type, iterable); + Py_END_CRITICAL_SECTION(); + return rv; } Py_ssize_t @@ -2441,48 +2451,72 @@ PySet_Size(PyObject *anyset) PyErr_BadInternalCall(); return -1; } - return PySet_GET_SIZE(anyset); + return set_len((PySetObject *)anyset); } int PySet_Clear(PyObject *set) { + int rv; + if (!PySet_Check(set)) { PyErr_BadInternalCall(); return -1; } - return set_clear_internal((PySetObject *)set); + + Py_BEGIN_CRITICAL_SECTION(set); + rv = set_clear_internal((PySetObject *)set); + Py_END_CRITICAL_SECTION(); + return rv; } int PySet_Contains(PyObject *anyset, PyObject *key) { + int rv; + if (!PyAnySet_Check(anyset)) { PyErr_BadInternalCall(); return -1; } - return set_contains_key((PySetObject *)anyset, key); + + Py_BEGIN_CRITICAL_SECTION(anyset); + rv = set_contains_key((PySetObject *)anyset, key); + Py_END_CRITICAL_SECTION(); + return rv; } int PySet_Discard(PyObject *set, PyObject *key) { + int rv; + if (!PySet_Check(set)) { PyErr_BadInternalCall(); return -1; } - return set_discard_key((PySetObject *)set, key); + + Py_BEGIN_CRITICAL_SECTION(set); + rv = set_discard_key((PySetObject *)set, key); + Py_END_CRITICAL_SECTION(); + return rv; } int PySet_Add(PyObject *anyset, PyObject *key) { + int rv; + if (!PySet_Check(anyset) && (!PyFrozenSet_Check(anyset) || Py_REFCNT(anyset) != 1)) { PyErr_BadInternalCall(); return -1; } - return set_add_key((PySetObject *)anyset, key); + + Py_BEGIN_CRITICAL_SECTION(anyset); + rv = set_add_key((PySetObject *)anyset, key); + Py_END_CRITICAL_SECTION(); + return rv; } // TODO: Make thread-safe in free-threaded builds @@ -2509,17 +2543,24 @@ PySet_Pop(PyObject *set) PyErr_BadInternalCall(); return NULL; } + // set_pop is guarded by @critical_section return set_pop((PySetObject *)set, NULL); } int _PySet_Update(PyObject *set, PyObject *iterable) { + int rv; + if (!PySet_Check(set)) { PyErr_BadInternalCall(); return -1; } - return set_update_internal((PySetObject *)set, iterable); + + Py_BEGIN_CRITICAL_SECTION2(set, iterable); + rv = set_update_internal((PySetObject *)set, iterable); + Py_END_CRITICAL_SECTION2(); + return rv; } /* Exported for the gdb plugin's benefit. */ From b06292ad16bff4048906ebf91149492b91c8da23 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 11 Feb 2024 16:30:44 +0100 Subject: [PATCH 06/26] Convert all `tp_` methods --- Objects/setobject.c | 45 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index f86f7f1344794e..0ca7c304704eaf 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -526,18 +526,22 @@ static PyObject * set_repr(PySetObject *so) { PyObject *result=NULL, *keys, *listrepr, *tmp; + + Py_BEGIN_CRITICAL_SECTION(so); int status = Py_ReprEnter((PyObject*)so); if (status != 0) { - if (status < 0) - return NULL; - return PyUnicode_FromFormat("%s(...)", Py_TYPE(so)->tp_name); + if (status < 0) { + goto done; + } + result = PyUnicode_FromFormat("%s(...)", Py_TYPE(so)->tp_name); + goto done; } /* shortcut for the empty set */ if (!so->used) { - Py_ReprLeave((PyObject*)so); - return PyUnicode_FromFormat("%s()", Py_TYPE(so)->tp_name); + result = PyUnicode_FromFormat("%s()", Py_TYPE(so)->tp_name); + goto done; } keys = PySequence_List((PyObject *)so); @@ -564,6 +568,7 @@ set_repr(PySetObject *so) Py_DECREF(listrepr); done: Py_ReprLeave((PyObject*)so); + Py_END_CRITICAL_SECTION(); return result; } @@ -2019,6 +2024,17 @@ set_contains(PySetObject *so, PyObject *key) return rv; } +static int +set_sq_contains(PySetObject *so, PyObject *key) +{ + int rv; + + Py_BEGIN_CRITICAL_SECTION(so); + rv = set_contains(so, key); + Py_END_CRITICAL_SECTION(); + return rv; +} + /*[clinic input] @critical_section @coexist @@ -2172,17 +2188,26 @@ static int set_init(PySetObject *self, PyObject *args, PyObject *kwds) { PyObject *iterable = NULL; + int rv; if (!_PyArg_NoKeywords("set", kwds)) return -1; if (!PyArg_UnpackTuple(args, Py_TYPE(self)->tp_name, 0, 1, &iterable)) return -1; + + Py_BEGIN_CRITICAL_SECTION(self); if (self->fill) set_clear_internal(self); self->hash = -1; + Py_END_CRITICAL_SECTION(); + if (iterable == NULL) return 0; - return set_update_internal(self, iterable); + + Py_BEGIN_CRITICAL_SECTION2(self, iterable); + rv = set_update_internal(self, iterable); + Py_END_CRITICAL_SECTION2(); + return rv; } static PyObject* @@ -2201,7 +2226,11 @@ set_vectorcall(PyObject *type, PyObject * const*args, } if (nargs) { - return make_new_set(_PyType_CAST(type), args[0]); + PyObject *rv; + Py_BEGIN_CRITICAL_SECTION(args[0]); + rv = make_new_set(_PyType_CAST(type), args[0]); + Py_END_CRITICAL_SECTION(); + return rv; } return make_new_set(_PyType_CAST(type), NULL); @@ -2215,7 +2244,7 @@ static PySequenceMethods set_as_sequence = { 0, /* sq_slice */ 0, /* sq_ass_item */ 0, /* sq_ass_slice */ - (objobjproc)set_contains, /* sq_contains */ + (objobjproc)set_sq_contains, /* sq_contains */ }; /* set object ********************************************************/ From 44694c095f6738a4959b0a7e61614df167499024 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 11 Feb 2024 17:07:08 +0100 Subject: [PATCH 07/26] Convert all number methods --- Objects/setobject.c | 71 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 11 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 0ca7c304704eaf..172ca983bc628f 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1217,31 +1217,43 @@ set_union_impl(PySetObject *so, PyObject *args) static PyObject * set_or(PySetObject *so, PyObject *other) { - PySetObject *result; + PySetObject *result = NULL; if (!PyAnySet_Check(so) || !PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; + Py_BEGIN_CRITICAL_SECTION2(so, other); result = (PySetObject *)set_copy(so, NULL); - if (result == NULL) - return NULL; - if ((PyObject *)so == other) - return (PyObject *)result; + if (result == NULL) { + goto done; + } + if ((PyObject *)so == other) { + goto done; + } if (set_update_internal(result, other)) { Py_DECREF(result); - return NULL; + result = NULL; } +done: + Py_END_CRITICAL_SECTION2(); return (PyObject *)result; } static PyObject * set_ior(PySetObject *so, PyObject *other) { + int rv; + if (!PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; - if (set_update_internal(so, other)) + Py_BEGIN_CRITICAL_SECTION2(so, other); + rv = set_update_internal(so, other); + Py_END_CRITICAL_SECTION2(); + + if(rv) { return NULL; + } return Py_NewRef(so); } @@ -1398,9 +1410,16 @@ set_intersection_update_multi_impl(PySetObject *so, PyObject *args) static PyObject * set_and(PySetObject *so, PyObject *other) { + PyObject *rv; + if (!PyAnySet_Check(so) || !PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; - return set_intersection(so, other); + + Py_BEGIN_CRITICAL_SECTION2(so, other); + rv = set_intersection(so, other); + Py_END_CRITICAL_SECTION2(); + + return rv; } static PyObject * @@ -1410,7 +1429,11 @@ set_iand(PySetObject *so, PyObject *other) if (!PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; + + Py_BEGIN_CRITICAL_SECTION2(so, other); result = set_intersection_update(so, other); + Py_END_CRITICAL_SECTION2(); + if (result == NULL) return NULL; Py_DECREF(result); @@ -1695,18 +1718,33 @@ set_difference_multi_impl(PySetObject *so, PyObject *args) static PyObject * set_sub(PySetObject *so, PyObject *other) { + PyObject *rv; + if (!PyAnySet_Check(so) || !PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; - return set_difference(so, other); + + Py_BEGIN_CRITICAL_SECTION2(so, other); + rv = set_difference(so, other); + Py_END_CRITICAL_SECTION2(); + + return rv; } static PyObject * set_isub(PySetObject *so, PyObject *other) { + int rv; + if (!PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; - if (set_difference_update_internal(so, other)) + + Py_BEGIN_CRITICAL_SECTION2(so, other); + rv = set_difference_update_internal(so, other); + Py_END_CRITICAL_SECTION2(); + + if(rv) { return NULL; + } return Py_NewRef(so); } @@ -1834,9 +1872,16 @@ set_symmetric_difference_impl(PySetObject *so, PyObject *other) static PyObject * set_xor(PySetObject *so, PyObject *other) { + PyObject *rv; + if (!PyAnySet_Check(so) || !PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; - return set_symmetric_difference(so, other); + + Py_BEGIN_CRITICAL_SECTION2(so, other); + rv = set_symmetric_difference(so, other); + Py_END_CRITICAL_SECTION2(); + + return rv; } static PyObject * @@ -1846,7 +1891,11 @@ set_ixor(PySetObject *so, PyObject *other) if (!PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; + + Py_BEGIN_CRITICAL_SECTION2(so, other); result = set_symmetric_difference_update(so, other); + Py_END_CRITICAL_SECTION2(); + if (result == NULL) return NULL; Py_DECREF(result); From ffe42730c5b6648b6fa96160205d13ac244a0a99 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 11 Feb 2024 17:24:13 +0100 Subject: [PATCH 08/26] Convert methods that take `*args` --- Objects/setobject.c | 53 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 9 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 172ca983bc628f..531cd443020c41 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -979,7 +979,11 @@ set_update_impl(PySetObject *so, PyObject *args) for (i=0 ; i Date: Fri, 12 Jan 2024 10:41:23 +0100 Subject: [PATCH 09/26] Add news entry --- .../2024-01-12-10-41-11.gh-issue-112069.e8MX9U.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-01-12-10-41-11.gh-issue-112069.e8MX9U.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-12-10-41-11.gh-issue-112069.e8MX9U.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-12-10-41-11.gh-issue-112069.e8MX9U.rst new file mode 100644 index 00000000000000..39137a8fbe5967 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-01-12-10-41-11.gh-issue-112069.e8MX9U.rst @@ -0,0 +1 @@ +Make sets thread-safe without the GIL From 9da7d2c105064ef1ec2c0e028531f076f498919e Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 13 Feb 2024 21:33:16 +0100 Subject: [PATCH 10/26] Move critical section to `make_new_set` --- Objects/setobject.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 531cd443020c41..571f67c947bc24 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1014,7 +1014,12 @@ make_new_set(PyTypeObject *type, PyObject *iterable) so->weakreflist = NULL; if (iterable != NULL) { - if (set_update_internal(so, iterable)) { + int rv; + + Py_BEGIN_CRITICAL_SECTION(iterable); + rv = set_update_internal(so, iterable); + Py_END_CRITICAL_SECTION(); + if (rv) { Py_DECREF(so); return NULL; } @@ -2538,23 +2543,13 @@ PyTypeObject PyFrozenSet_Type = { PyObject * PySet_New(PyObject *iterable) { - PyObject *rv; - - Py_BEGIN_CRITICAL_SECTION(iterable); - rv = make_new_set(&PySet_Type, iterable); - Py_END_CRITICAL_SECTION(); - return rv; + return make_new_set(&PySet_Type, iterable); } PyObject * PyFrozenSet_New(PyObject *iterable) { - PyObject *rv; - - Py_BEGIN_CRITICAL_SECTION(iterable); - rv = make_new_set(&PyFrozenSet_Type, iterable); - Py_END_CRITICAL_SECTION(); - return rv; + return make_new_set(&PyFrozenSet_Type, iterable); } Py_ssize_t From ec7cfff8b73ea2ba3464c2c56ddcf26ffb6e93d6 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 13 Feb 2024 21:34:23 +0100 Subject: [PATCH 11/26] Align comments --- Objects/setobject.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 571f67c947bc24..f667aa87b3fa32 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2326,14 +2326,14 @@ set_vectorcall(PyObject *type, PyObject * const*args, } static PySequenceMethods set_as_sequence = { - (lenfunc)set_len, /* sq_length */ + (lenfunc)set_len, /* sq_length */ 0, /* sq_concat */ 0, /* sq_repeat */ 0, /* sq_item */ 0, /* sq_slice */ 0, /* sq_ass_item */ 0, /* sq_ass_slice */ - (objobjproc)set_sq_contains, /* sq_contains */ + (objobjproc)set_sq_contains, /* sq_contains */ }; /* set object ********************************************************/ From 65b9712d96e013dee8e06cd4e7c315d7cd548ee8 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 13 Feb 2024 21:36:36 +0100 Subject: [PATCH 12/26] Remove locking from `set_vectorcall` --- Objects/setobject.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index f667aa87b3fa32..a21b2fa935d9db 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2315,11 +2315,7 @@ set_vectorcall(PyObject *type, PyObject * const*args, } if (nargs) { - PyObject *rv; - Py_BEGIN_CRITICAL_SECTION(args[0]); - rv = make_new_set(_PyType_CAST(type), args[0]); - Py_END_CRITICAL_SECTION(); - return rv; + return make_new_set(_PyType_CAST(type), args[0]); } return make_new_set(_PyType_CAST(type), NULL); From 31e6e479c6209689073fef6baac7a01b10e23422 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 13 Feb 2024 21:39:15 +0100 Subject: [PATCH 13/26] Simplify `PySet_Clear` --- Objects/setobject.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index a21b2fa935d9db..36239561a0e13b 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2561,17 +2561,12 @@ PySet_Size(PyObject *anyset) int PySet_Clear(PyObject *set) { - int rv; - if (!PySet_Check(set)) { PyErr_BadInternalCall(); return -1; } - - Py_BEGIN_CRITICAL_SECTION(set); - rv = set_clear_internal((PySetObject *)set); - Py_END_CRITICAL_SECTION(); - return rv; + set_clear((PySetObject *)set, NULL); + return 0; } int From ef76ff113d2d327d688cd54553355bbf3a43ff90 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 13 Feb 2024 21:50:58 +0100 Subject: [PATCH 14/26] Remove locking around `set_copy` --- Objects/setobject.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 36239561a0e13b..f1e33fad8bfd8b 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1207,9 +1207,7 @@ set_union_impl(PySetObject *so, PyObject *args) PyObject *other; Py_ssize_t i; - Py_BEGIN_CRITICAL_SECTION(so); result = (PySetObject *)set_copy(so, NULL); - Py_END_CRITICAL_SECTION(); if (result == NULL) return NULL; @@ -1232,24 +1230,23 @@ set_union_impl(PySetObject *so, PyObject *args) static PyObject * set_or(PySetObject *so, PyObject *other) { - PySetObject *result = NULL; + PySetObject *result; if (!PyAnySet_Check(so) || !PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; - Py_BEGIN_CRITICAL_SECTION2(so, other); result = (PySetObject *)set_copy(so, NULL); if (result == NULL) { - goto done; + return NULL; } if ((PyObject *)so == other) { - goto done; + return (PyObject *)result; } + Py_BEGIN_CRITICAL_SECTION2(so, other); if (set_update_internal(result, other)) { Py_DECREF(result); result = NULL; } -done: Py_END_CRITICAL_SECTION2(); return (PyObject *)result; } @@ -1371,11 +1368,7 @@ set_intersection_multi_impl(PySetObject *so, PyObject *args) Py_ssize_t i; if (PyTuple_GET_SIZE(args) == 0) { - PyObject *result; - Py_BEGIN_CRITICAL_SECTION(so); - result = set_copy(so, NULL); - Py_END_CRITICAL_SECTION(); - return result; + return set_copy(so, NULL); } PyObject *result = Py_NewRef(so); @@ -1728,10 +1721,7 @@ set_difference_multi_impl(PySetObject *so, PyObject *args) PyObject *result, *other; if (PyTuple_GET_SIZE(args) == 0) { - Py_BEGIN_CRITICAL_SECTION(so); - result = set_copy(so, NULL); - Py_END_CRITICAL_SECTION(); - return result; + return set_copy(so, NULL); } other = PyTuple_GET_ITEM(args, 0); From 9305a4cea8a036ead78fa979008b6ba1d84a8bb0 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 13 Feb 2024 21:55:46 +0100 Subject: [PATCH 15/26] Remove unnecessary locking --- Objects/setobject.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index f1e33fad8bfd8b..eb59839a2f710b 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1902,16 +1902,9 @@ set_symmetric_difference_impl(PySetObject *so, PyObject *other) static PyObject * set_xor(PySetObject *so, PyObject *other) { - PyObject *rv; - if (!PyAnySet_Check(so) || !PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; - - Py_BEGIN_CRITICAL_SECTION2(so, other); - rv = set_symmetric_difference(so, other); - Py_END_CRITICAL_SECTION2(); - - return rv; + return set_symmetric_difference(so, other); } static PyObject * @@ -1921,11 +1914,7 @@ set_ixor(PySetObject *so, PyObject *other) if (!PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; - - Py_BEGIN_CRITICAL_SECTION2(so, other); result = set_symmetric_difference_update(so, other); - Py_END_CRITICAL_SECTION2(); - if (result == NULL) return NULL; Py_DECREF(result); @@ -2632,7 +2621,6 @@ PySet_Pop(PyObject *set) PyErr_BadInternalCall(); return NULL; } - // set_pop is guarded by @critical_section return set_pop((PySetObject *)set, NULL); } From e2271e3d9da74ceb1bebd15a72b761983f87b18b Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 13 Feb 2024 22:11:24 +0100 Subject: [PATCH 16/26] Remove unnecessary locking around dict objects --- Objects/setobject.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index eb59839a2f710b..2ccd4de7c97433 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -931,17 +931,12 @@ set_update_internal(PySetObject *so, PyObject *other) if (set_table_resize(so, (so->used + dictsize)*2) != 0) return -1; } - int err = 0; - Py_BEGIN_CRITICAL_SECTION(other); while (_PyDict_Next(other, &pos, &key, &value, &hash)) { if (set_add_entry(so, key, hash)) { - err = -1; - goto exit; + return -1; } } -exit: - Py_END_CRITICAL_SECTION(); - return err; + return 0; } it = PyObject_GetIter(other); @@ -1830,13 +1825,7 @@ set_symmetric_difference_update_impl(PySetObject *so, PyObject *other) return set_clear(so, NULL); if (PyDict_CheckExact(other)) { - PyObject *res; - - Py_BEGIN_CRITICAL_SECTION(other); - res = set_symmetric_difference_update_dict(so, other); - Py_END_CRITICAL_SECTION(); - - return res; + return set_symmetric_difference_update_dict(so, other); } if (PyAnySet_Check(other)) { From 59e4a231f2638be0a1c69017348424327d97b882 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 13 Feb 2024 23:30:37 +0100 Subject: [PATCH 17/26] Assert that the lock is held in `set_update_internal` --- Objects/setobject.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Objects/setobject.c b/Objects/setobject.c index 2ccd4de7c97433..c15d8977d26ea1 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -54,6 +54,21 @@ class setobject_converter(self_converter): [python start generated code]*/ /*[python end generated code: output=da39a3ee5e6b4b0d input=33a44506d4d57793]*/ +#ifdef Py_GIL_DISABLED + +static inline void +ASSERT_SET_LOCKED(PyObject *op) +{ + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); +} +#define ASSERT_SET_LOCKED(op) ASSERT_SET_LOCKED(_Py_CAST(PyObject*, op)) + +#else + +#define ASSERT_SET_LOCKED(op) + +#endif + /* Object used as dummy key to fill deleted entries */ static PyObject _dummy_struct; @@ -912,6 +927,9 @@ set_update_internal(PySetObject *so, PyObject *other) { PyObject *key, *it; + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(other); + if (PyAnySet_Check(other)) return set_merge(so, other); From 3b4fa1f07696e000431603d039446b8a0698180a Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Tue, 13 Feb 2024 23:56:47 +0100 Subject: [PATCH 18/26] Remove unnecessary macros --- Objects/setobject.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index c15d8977d26ea1..e039822be6a103 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -54,21 +54,6 @@ class setobject_converter(self_converter): [python start generated code]*/ /*[python end generated code: output=da39a3ee5e6b4b0d input=33a44506d4d57793]*/ -#ifdef Py_GIL_DISABLED - -static inline void -ASSERT_SET_LOCKED(PyObject *op) -{ - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); -} -#define ASSERT_SET_LOCKED(op) ASSERT_SET_LOCKED(_Py_CAST(PyObject*, op)) - -#else - -#define ASSERT_SET_LOCKED(op) - -#endif - /* Object used as dummy key to fill deleted entries */ static PyObject _dummy_struct; From 8d76137c17ce9faab51edfe308551821527a2c8c Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 6 Mar 2024 23:17:26 +0000 Subject: [PATCH 19/26] Avoid critical section reacquisitions --- Objects/clinic/setobject.c.h | 17 +- Objects/setobject.c | 322 +++++++++++++++++++++-------------- 2 files changed, 198 insertions(+), 141 deletions(-) diff --git a/Objects/clinic/setobject.c.h b/Objects/clinic/setobject.c.h index a0b7bd7084dc29..3853ce3bce685b 100644 --- a/Objects/clinic/setobject.c.h +++ b/Objects/clinic/setobject.c.h @@ -346,21 +346,6 @@ PyDoc_STRVAR(set_symmetric_difference_update__doc__, #define SET_SYMMETRIC_DIFFERENCE_UPDATE_METHODDEF \ {"symmetric_difference_update", (PyCFunction)set_symmetric_difference_update, METH_O, set_symmetric_difference_update__doc__}, -static PyObject * -set_symmetric_difference_update_impl(PySetObject *so, PyObject *other); - -static PyObject * -set_symmetric_difference_update(PySetObject *so, PyObject *other) -{ - PyObject *return_value = NULL; - - Py_BEGIN_CRITICAL_SECTION2(so, other); - return_value = set_symmetric_difference_update_impl(so, other); - Py_END_CRITICAL_SECTION2(); - - return return_value; -} - PyDoc_STRVAR(set_symmetric_difference__doc__, "symmetric_difference($self, other, /)\n" "--\n" @@ -583,4 +568,4 @@ set___sizeof__(PySetObject *so, PyObject *Py_UNUSED(ignored)) return return_value; } -/*[clinic end generated code: output=e320fd760fa48265 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=de4ee725bd29f758 input=a9049054013a1b77]*/ diff --git a/Objects/setobject.c b/Objects/setobject.c index beb8c3dd122048..32e65dd616d3ae 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -130,6 +130,8 @@ set_add_entry(PySetObject *so, PyObject *key, Py_hash_t hash) int probes; int cmp; + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so); + /* Pre-increment is necessary to prevent arbitrary code in the rich comparison from deallocating the key just before the insertion. */ Py_INCREF(key); @@ -579,7 +581,7 @@ set_len(PySetObject *so) } static int -set_merge(PySetObject *so, PyObject *otherset) +set_merge_lock_held(PySetObject *so, PyObject *otherset) { PySetObject *other; PyObject *key; @@ -589,6 +591,8 @@ set_merge(PySetObject *so, PyObject *otherset) assert (PyAnySet_Check(so)); assert (PyAnySet_Check(otherset)); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(otherset); other = (PySetObject*)otherset; if (other == so || other->used == 0) @@ -908,44 +912,46 @@ set_iter(PySetObject *so) } static int -set_update_internal(PySetObject *so, PyObject *other) +set_update_dict_lock_held(PySetObject *so, PyObject *other) { - PyObject *key, *it; + assert(PyDict_CheckExact(other)); _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so); _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(other); - if (PyAnySet_Check(other)) - return set_merge(so, other); - - if (PyDict_CheckExact(other)) { - PyObject *value; - Py_ssize_t pos = 0; - Py_hash_t hash; - Py_ssize_t dictsize = PyDict_GET_SIZE(other); + /* Do one big resize at the start, rather than + * incrementally resizing as we insert new keys. Expect + * that there will be no (or few) overlapping keys. + */ + Py_ssize_t dictsize = PyDict_GET_SIZE(other); + if ((so->fill + dictsize)*5 >= so->mask*3) { + if (set_table_resize(so, (so->used + dictsize)*2) != 0) + return -1; + } - /* Do one big resize at the start, rather than - * incrementally resizing as we insert new keys. Expect - * that there will be no (or few) overlapping keys. - */ - if (dictsize < 0) + Py_ssize_t pos = 0; + PyObject *key; + PyObject *value; + Py_hash_t hash; + while (_PyDict_Next(other, &pos, &key, &value, &hash)) { + if (set_add_entry(so, key, hash)) { return -1; - if ((so->fill + dictsize)*5 >= so->mask*3) { - if (set_table_resize(so, (so->used + dictsize)*2) != 0) - return -1; } - while (_PyDict_Next(other, &pos, &key, &value, &hash)) { - if (set_add_entry(so, key, hash)) { - return -1; - } - } - return 0; } + return 0; +} - it = PyObject_GetIter(other); - if (it == NULL) +static int +set_update_iterable_lock_held(PySetObject *so, PyObject *other) +{ + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so); + + PyObject *it = PyObject_GetIter(other); + if (it == NULL) { return -1; + } + PyObject *key; while ((key = PyIter_Next(it)) != NULL) { if (set_add_key(so, key)) { Py_DECREF(it); @@ -960,6 +966,73 @@ set_update_internal(PySetObject *so, PyObject *other) return 0; } +static int +set_update_lock_held(PySetObject *so, PyObject *other) +{ + if (PyAnySet_Check(other)) { + return set_merge_lock_held(so, other); + } + else if (PyDict_CheckExact(other)) { + return set_update_dict_lock_held(so, other); + } + else { + return set_update_iterable_lock_held(so, other); + } +} + +// set_update for a `so` that is only visible to the current thread +static int +set_update_local(PySetObject *so, PyObject *other) +{ + assert(Py_REFCNT(so) == 1); + if (PyAnySet_Check(other)) { + int rv; + Py_BEGIN_CRITICAL_SECTION(other); + rv = set_merge_lock_held(so, other); + Py_END_CRITICAL_SECTION(); + return rv; + } + else if (PyDict_CheckExact(other)) { + int rv; + Py_BEGIN_CRITICAL_SECTION(other); + rv = set_update_dict_lock_held(so, other); + Py_END_CRITICAL_SECTION(); + return rv; + } + else { + return set_update_iterable_lock_held(so, other); + } +} + +static int +set_update_internal(PySetObject *so, PyObject *other) +{ + if (PyAnySet_Check(other)) { + if ((PyObject *)so == other) { + return 0; + } + int rv; + Py_BEGIN_CRITICAL_SECTION2(so, other); + rv = set_merge_lock_held(so, other); + Py_END_CRITICAL_SECTION2(); + return rv; + } + else if (PyDict_CheckExact(other)) { + int rv; + Py_BEGIN_CRITICAL_SECTION2(so, other); + rv = set_update_dict_lock_held(so, other); + Py_END_CRITICAL_SECTION2(); + return rv; + } + else { + int rv; + Py_BEGIN_CRITICAL_SECTION(so); + rv = set_update_iterable_lock_held(so, other); + Py_END_CRITICAL_SECTION(); + return rv; + } +} + /*[clinic input] set.update so: setobject @@ -977,11 +1050,7 @@ set_update_impl(PySetObject *so, PyObject *args) for (i=0 ; iweakreflist = NULL; if (iterable != NULL) { - int rv; - - Py_BEGIN_CRITICAL_SECTION(iterable); - rv = set_update_internal(so, iterable); - Py_END_CRITICAL_SECTION(); - if (rv) { + if (set_update_local(so, iterable)) { Py_DECREF(so); return NULL; } @@ -1151,7 +1215,16 @@ static PyObject * set_copy_impl(PySetObject *so) /*[clinic end generated code: output=c9223a1e1cc6b041 input=c169a4fbb8209257]*/ { - return make_new_set_basetype(Py_TYPE(so), (PyObject *)so); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so); + PyObject *copy = make_new_set_basetype(Py_TYPE(so), NULL); + if (copy == NULL) { + return NULL; + } + if (set_merge_lock_held((PySetObject *)copy, (PyObject *)so) < 0) { + Py_DECREF(copy); + return NULL; + } + return copy; } /*[clinic input] @@ -1169,7 +1242,7 @@ frozenset_copy_impl(PySetObject *so) if (PyFrozenSet_CheckExact(so)) { return Py_NewRef(so); } - return set_copy(so, NULL); + return set_copy_impl(so); } /*[clinic input] @@ -1210,14 +1283,10 @@ set_union_impl(PySetObject *so, PyObject *args) return NULL; for (i=0 ; ikey; + Py_hash_t hash = entry->hash; + Py_INCREF(key); + int rv = set_discard_entry(so, key, hash); + if (rv < 0) { + Py_DECREF(key); + return -1; + } + if (rv == DISCARD_NOTFOUND) { + if (set_add_entry(so, key, hash)) { + Py_DECREF(key); + return -1; + } + } + Py_DECREF(key); + } + return 0; } /*[clinic input] -@critical_section so other set.symmetric_difference_update so: setobject other: object @@ -1814,51 +1905,39 @@ Update the set, keeping only elements found in either set, but not in both. [clinic start generated code]*/ static PyObject * -set_symmetric_difference_update_impl(PySetObject *so, PyObject *other) -/*[clinic end generated code: output=79f80b4ee5da66c1 input=34c143ee8c2ba08b]*/ +set_symmetric_difference_update(PySetObject *so, PyObject *other) +/*[clinic end generated code: output=fbb049c0806028de input=a50acf0365e1f0a5]*/ { - PySetObject *otherset; - PyObject *key; - Py_ssize_t pos = 0; - Py_hash_t hash; - setentry *entry; - int rv; - - if ((PyObject *)so == other) + if ((PyObject *)so == other) { return set_clear(so, NULL); + } + int rv; if (PyDict_CheckExact(other)) { - return set_symmetric_difference_update_dict(so, other); + Py_BEGIN_CRITICAL_SECTION2(so, other); + rv = set_symmetric_difference_update_dict(so, other); + Py_END_CRITICAL_SECTION2(); } - - if (PyAnySet_Check(other)) { - otherset = (PySetObject *)Py_NewRef(other); - } else { - otherset = (PySetObject *)make_new_set_basetype(Py_TYPE(so), other); - if (otherset == NULL) - return NULL; + else if (PyAnySet_Check(other)) { + Py_BEGIN_CRITICAL_SECTION2(so, other); + rv = set_symmetric_difference_update_set(so, (PySetObject *)other); + Py_END_CRITICAL_SECTION2(); } - - while (set_next(otherset, &pos, &entry)) { - key = entry->key; - hash = entry->hash; - Py_INCREF(key); - rv = set_discard_entry(so, key, hash); - if (rv < 0) { - Py_DECREF(otherset); - Py_DECREF(key); + else { + PySetObject *otherset = (PySetObject *)make_new_set_basetype(Py_TYPE(so), other); + if (otherset == NULL) { return NULL; } - if (rv == DISCARD_NOTFOUND) { - if (set_add_entry(so, key, hash)) { - Py_DECREF(otherset); - Py_DECREF(key); - return NULL; - } - } - Py_DECREF(key); + + Py_BEGIN_CRITICAL_SECTION(so); + rv = set_symmetric_difference_update_set(so, otherset); + Py_END_CRITICAL_SECTION(); + + Py_DECREF(otherset); + } + if (rv < 0) { + return NULL; } - Py_DECREF(otherset); Py_RETURN_NONE; } @@ -1876,19 +1955,19 @@ static PyObject * set_symmetric_difference_impl(PySetObject *so, PyObject *other) /*[clinic end generated code: output=270ee0b5d42b0797 input=624f6e7bbdf70db1]*/ { - PyObject *rv; - PySetObject *otherset; - - otherset = (PySetObject *)make_new_set_basetype(Py_TYPE(so), other); - if (otherset == NULL) + PySetObject *result = (PySetObject *)make_new_set_basetype(Py_TYPE(so), NULL); + if (result == NULL) { return NULL; - rv = set_symmetric_difference_update(otherset, (PyObject *)so); - if (rv == NULL) { - Py_DECREF(otherset); + } + if (set_update_lock_held(result, other) < 0) { + Py_DECREF(result); + return NULL; + } + if (set_symmetric_difference_update_set(result, so) < 0) { + Py_DECREF(result); return NULL; } - Py_DECREF(rv); - return (PyObject *)otherset; + return (PyObject *)result; } static PyObject * @@ -2065,7 +2144,7 @@ set_add_impl(PySetObject *so, PyObject *key) } static int -set_contains_holds_lock(PySetObject *so, PyObject *key) +set_contains_lock_held(PySetObject *so, PyObject *key) { PyObject *tmpkey; int rv; @@ -2089,7 +2168,7 @@ _PySet_Contains(PySetObject *so, PyObject *key) { int rv; Py_BEGIN_CRITICAL_SECTION(so); - rv = set_contains_holds_lock(so, key); + rv = set_contains_lock_held(so, key); Py_END_CRITICAL_SECTION(); return rv; } @@ -2111,7 +2190,7 @@ set___contains___impl(PySetObject *so, PyObject *key) { long result; - result = _PySet_Contains(so, key); + result = set_contains_lock_held(so, key); if (result < 0) return NULL; return PyBool_FromLong(result); @@ -2263,9 +2342,7 @@ set_init(PySetObject *self, PyObject *args, PyObject *kwds) if (iterable == NULL) return 0; - Py_BEGIN_CRITICAL_SECTION2(self, iterable); rv = set_update_internal(self, iterable); - Py_END_CRITICAL_SECTION2(); return rv; } @@ -2618,17 +2695,12 @@ PySet_Pop(PyObject *set) int _PySet_Update(PyObject *set, PyObject *iterable) { - int rv; - if (!PySet_Check(set)) { PyErr_BadInternalCall(); return -1; } - Py_BEGIN_CRITICAL_SECTION2(set, iterable); - rv = set_update_internal((PySetObject *)set, iterable); - Py_END_CRITICAL_SECTION2(); - return rv; + return set_update_internal((PySetObject *)set, iterable); } /* Exported for the gdb plugin's benefit. */ From 255d0ab154d2a406682628ff9b257c596856e680 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 7 Mar 2024 19:04:46 +0000 Subject: [PATCH 20/26] Split critical section off of set_repr --- Objects/setobject.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 32e65dd616d3ae..c1f19e062ec750 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -525,25 +525,21 @@ set_dealloc(PySetObject *so) } static PyObject * -set_repr(PySetObject *so) +set_repr_lock_held(PySetObject *so) { PyObject *result=NULL, *keys, *listrepr, *tmp; - - Py_BEGIN_CRITICAL_SECTION(so); int status = Py_ReprEnter((PyObject*)so); if (status != 0) { - if (status < 0) { - goto done; - } - result = PyUnicode_FromFormat("%s(...)", Py_TYPE(so)->tp_name); - goto done; + if (status < 0) + return NULL; + return PyUnicode_FromFormat("%s(...)", Py_TYPE(so)->tp_name); } /* shortcut for the empty set */ if (!so->used) { - result = PyUnicode_FromFormat("%s()", Py_TYPE(so)->tp_name); - goto done; + Py_ReprLeave((PyObject*)so); + return PyUnicode_FromFormat("%s()", Py_TYPE(so)->tp_name); } keys = PySequence_List((PyObject *)so); @@ -570,6 +566,15 @@ set_repr(PySetObject *so) Py_DECREF(listrepr); done: Py_ReprLeave((PyObject*)so); + return result; +} + +static PyObject * +set_repr(PySetObject *so) +{ + PyObject *result; + Py_BEGIN_CRITICAL_SECTION(so); + result = set_repr_lock_held(so); Py_END_CRITICAL_SECTION(); return result; } From 4dc853ddf4f51a2d95085b29585b7a641d98f1c8 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 7 Mar 2024 19:14:23 +0000 Subject: [PATCH 21/26] Minor formatting changes --- Objects/setobject.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index c1f19e062ec750..eee90351b1ed82 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2331,9 +2331,8 @@ static int set_init(PySetObject *self, PyObject *args, PyObject *kwds) { PyObject *iterable = NULL; - int rv; - if (!_PyArg_NoKeywords("set", kwds)) + if (!_PyArg_NoKeywords("set", kwds)) return -1; if (!PyArg_UnpackTuple(args, Py_TYPE(self)->tp_name, 0, 1, &iterable)) return -1; @@ -2346,9 +2345,7 @@ set_init(PySetObject *self, PyObject *args, PyObject *kwds) if (iterable == NULL) return 0; - - rv = set_update_internal(self, iterable); - return rv; + return set_update_internal(self, iterable); } static PyObject* @@ -2624,13 +2621,12 @@ PySet_Clear(PyObject *set) int PySet_Contains(PyObject *anyset, PyObject *key) { - int rv; - if (!PyAnySet_Check(anyset)) { PyErr_BadInternalCall(); return -1; } + int rv; Py_BEGIN_CRITICAL_SECTION(anyset); rv = set_contains_key((PySetObject *)anyset, key); Py_END_CRITICAL_SECTION(); @@ -2640,13 +2636,12 @@ PySet_Contains(PyObject *anyset, PyObject *key) int PySet_Discard(PyObject *set, PyObject *key) { - int rv; - if (!PySet_Check(set)) { PyErr_BadInternalCall(); return -1; } + int rv; Py_BEGIN_CRITICAL_SECTION(set); rv = set_discard_key((PySetObject *)set, key); Py_END_CRITICAL_SECTION(); @@ -2656,14 +2651,13 @@ PySet_Discard(PyObject *set, PyObject *key) int PySet_Add(PyObject *anyset, PyObject *key) { - int rv; - if (!PySet_Check(anyset) && (!PyFrozenSet_Check(anyset) || Py_REFCNT(anyset) != 1)) { PyErr_BadInternalCall(); return -1; } + int rv; Py_BEGIN_CRITICAL_SECTION(anyset); rv = set_add_key((PySetObject *)anyset, key); Py_END_CRITICAL_SECTION(); @@ -2704,7 +2698,6 @@ _PySet_Update(PyObject *set, PyObject *iterable) PyErr_BadInternalCall(); return -1; } - return set_update_internal((PySetObject *)set, iterable); } From ad235887f9d37c7fb132ae671ba760a35665aec4 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 7 Mar 2024 19:16:33 +0000 Subject: [PATCH 22/26] Remove NEWS entry --- .../2024-01-12-10-41-11.gh-issue-112069.e8MX9U.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-01-12-10-41-11.gh-issue-112069.e8MX9U.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-12-10-41-11.gh-issue-112069.e8MX9U.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-12-10-41-11.gh-issue-112069.e8MX9U.rst deleted file mode 100644 index 39137a8fbe5967..00000000000000 --- a/Misc/NEWS.d/next/Core and Builtins/2024-01-12-10-41-11.gh-issue-112069.e8MX9U.rst +++ /dev/null @@ -1 +0,0 @@ -Make sets thread-safe without the GIL From 0fde8dd4984353146a984786f8be655996023ff1 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 7 Mar 2024 17:34:33 -0500 Subject: [PATCH 23/26] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Objects/setobject.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index eee90351b1ed82..dfd7b40ff4f082 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -930,8 +930,9 @@ set_update_dict_lock_held(PySetObject *so, PyObject *other) */ Py_ssize_t dictsize = PyDict_GET_SIZE(other); if ((so->fill + dictsize)*5 >= so->mask*3) { - if (set_table_resize(so, (so->used + dictsize)*2) != 0) + if (set_table_resize(so, (so->used + dictsize)*2) != 0) { return -1; + } } Py_ssize_t pos = 0; @@ -980,9 +981,7 @@ set_update_lock_held(PySetObject *so, PyObject *other) else if (PyDict_CheckExact(other)) { return set_update_dict_lock_held(so, other); } - else { - return set_update_iterable_lock_held(so, other); - } + return set_update_iterable_lock_held(so, other); } // set_update for a `so` that is only visible to the current thread @@ -1004,16 +1003,14 @@ set_update_local(PySetObject *so, PyObject *other) Py_END_CRITICAL_SECTION(); return rv; } - else { - return set_update_iterable_lock_held(so, other); - } + return set_update_iterable_lock_held(so, other); } static int set_update_internal(PySetObject *so, PyObject *other) { if (PyAnySet_Check(other)) { - if ((PyObject *)so == other) { + if (Py_Is((PyObject *)so, other)) { return 0; } int rv; @@ -1311,7 +1308,7 @@ set_or(PySetObject *so, PyObject *other) if (result == NULL) { return NULL; } - if ((PyObject *)so == other) { + if (Py_Is((PyObject *)so, other)) { return (PyObject *)result; } if (set_update_local(result, other)) { @@ -1881,9 +1878,8 @@ set_symmetric_difference_update_set(PySetObject *so, PySetObject *other) Py_ssize_t pos = 0; setentry *entry; while (set_next(other, &pos, &entry)) { - PyObject *key = entry->key; + PyObject *key = Py_NewRef(entry->key); Py_hash_t hash = entry->hash; - Py_INCREF(key); int rv = set_discard_entry(so, key, hash); if (rv < 0) { Py_DECREF(key); @@ -1913,7 +1909,7 @@ static PyObject * set_symmetric_difference_update(PySetObject *so, PyObject *other) /*[clinic end generated code: output=fbb049c0806028de input=a50acf0365e1f0a5]*/ { - if ((PyObject *)so == other) { + if (Py_Is((PyObject *)so, other)) { return set_clear(so, NULL); } @@ -2614,7 +2610,7 @@ PySet_Clear(PyObject *set) PyErr_BadInternalCall(); return -1; } - set_clear((PySetObject *)set, NULL); + (void)set_clear((PySetObject *)set, NULL); return 0; } From 6573c8d7f20658b6baf5a0ba7054cc65e75b6626 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 7 Mar 2024 22:37:35 +0000 Subject: [PATCH 24/26] Move definitions of 'rv' down --- Objects/setobject.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index dfd7b40ff4f082..d72e6f773f128c 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1489,11 +1489,10 @@ set_intersection_update_multi_impl(PySetObject *so, PyObject *args) static PyObject * set_and(PySetObject *so, PyObject *other) { - PyObject *rv; - if (!PyAnySet_Check(so) || !PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; + PyObject *rv; Py_BEGIN_CRITICAL_SECTION2(so, other); rv = set_intersection(so, other); Py_END_CRITICAL_SECTION2(); @@ -1812,31 +1811,27 @@ set_difference_multi_impl(PySetObject *so, PyObject *args) static PyObject * set_sub(PySetObject *so, PyObject *other) { - PyObject *rv; - if (!PyAnySet_Check(so) || !PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; + PyObject *rv; Py_BEGIN_CRITICAL_SECTION2(so, other); rv = set_difference(so, other); Py_END_CRITICAL_SECTION2(); - return rv; } static PyObject * set_isub(PySetObject *so, PyObject *other) { - int rv; - if (!PyAnySet_Check(other)) Py_RETURN_NOTIMPLEMENTED; + int rv; Py_BEGIN_CRITICAL_SECTION2(so, other); rv = set_difference_update_internal(so, other); Py_END_CRITICAL_SECTION2(); - - if(rv) { + if (rv < 0) { return NULL; } return Py_NewRef(so); From 828a62d486413ada5324917eb4a0493cd0f215ef Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Fri, 8 Mar 2024 18:22:04 +0000 Subject: [PATCH 25/26] Use FT_ATOMIC_LOAD_SSIZE_RELAXED --- Objects/setobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index d72e6f773f128c..51254ef9a7ccc8 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -582,7 +582,7 @@ set_repr(PySetObject *so) static Py_ssize_t set_len(PySetObject *so) { - return _Py_atomic_load_ssize_relaxed(&so->used); + return FT_ATOMIC_LOAD_SSIZE_RELAXED(so->used); } static int From 95a91001a56413d9fe148fe4b4bb6e9811d91f30 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Fri, 8 Mar 2024 18:50:39 +0000 Subject: [PATCH 26/26] Add missing include --- Objects/setobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/setobject.c b/Objects/setobject.c index 51254ef9a7ccc8..592711f305cbaf 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -37,6 +37,7 @@ #include "pycore_dict.h" // _PyDict_Contains_KnownHash() #include "pycore_modsupport.h" // _PyArg_NoKwnames() #include "pycore_object.h" // _PyObject_GC_UNTRACK() +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_SSIZE_RELAXED() #include "pycore_pyerrors.h" // _PyErr_SetKeyError() #include "pycore_setobject.h" // _PySet_NextEntry() definition #include // offsetof()