https://github.com/python/cpython/commit/d9411ae3c2f6b59d97a0a16ad3dd148cc58ad943
commit: d9411ae3c2f6b59d97a0a16ad3dd148cc58ad943
branch: main
author: Dino Viehland <dinoviehl...@meta.com>
committer: DinoV <dinoviehl...@gmail.com>
date: 2025-03-21T15:58:32-07:00
summary:
gh-130312: SET_ADD should not lock (#130136)
SET_ADD should not lock
files:
M Include/internal/pycore_setobject.h
M Objects/setobject.c
M Python/bytecodes.c
M Python/executor_cases.c.h
M Python/generated_cases.c.h
diff --git a/Include/internal/pycore_setobject.h
b/Include/internal/pycore_setobject.h
index 0494c07fe1869d..24d0135ed1aeca 100644
--- a/Include/internal/pycore_setobject.h
+++ b/Include/internal/pycore_setobject.h
@@ -33,6 +33,8 @@ PyAPI_FUNC(int) _PySet_Contains(PySetObject *so, PyObject
*key);
// Clears the set without acquiring locks. Used by _PyCode_Fini.
extern void _PySet_ClearInternal(PySetObject *so);
+PyAPI_FUNC(int) _PySet_AddTakeRef(PySetObject *so, PyObject *key);
+
#ifdef __cplusplus
}
#endif
diff --git a/Objects/setobject.c b/Objects/setobject.c
index 9181bc27ac4e97..5ad83c3f1633b9 100644
--- a/Objects/setobject.c
+++ b/Objects/setobject.c
@@ -122,7 +122,7 @@ set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash)
static int set_table_resize(PySetObject *, Py_ssize_t);
static int
-set_add_entry(PySetObject *so, PyObject *key, Py_hash_t hash)
+set_add_entry_takeref(PySetObject *so, PyObject *key, Py_hash_t hash)
{
setentry *table;
setentry *freeslot;
@@ -133,12 +133,6 @@ 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);
-
restart:
mask = so->mask;
@@ -209,6 +203,27 @@ set_add_entry(PySetObject *so, PyObject *key, Py_hash_t
hash)
return -1;
}
+static int
+set_add_entry(PySetObject *so, PyObject *key, Py_hash_t hash)
+{
+ _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so);
+
+ return set_add_entry_takeref(so, Py_NewRef(key), hash);
+}
+
+int
+_PySet_AddTakeRef(PySetObject *so, PyObject *key)
+{
+ Py_hash_t hash = _PyObject_HashFast(key);
+ if (hash == -1) {
+ Py_DECREF(key);
+ return -1;
+ }
+ // We don't pre-increment here, the caller holds a strong
+ // reference to the object which we are stealing.
+ return set_add_entry_takeref(so, key, hash);
+}
+
/*
Internal routine used by set_table_resize() to insert an item which is
known to be absent from the set. Besides the performance benefit,
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index fe1465b36c7d04..0db4da013ff40a 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -975,9 +975,8 @@ dummy_func(
}
inst(SET_ADD, (set, unused[oparg-1], v -- set, unused[oparg-1])) {
- int err = PySet_Add(PyStackRef_AsPyObjectBorrow(set),
- PyStackRef_AsPyObjectBorrow(v));
- PyStackRef_CLOSE(v);
+ int err = _PySet_AddTakeRef((PySetObject
*)PyStackRef_AsPyObjectBorrow(set),
+ PyStackRef_AsPyObjectSteal(v));
ERROR_IF(err, error);
}
@@ -1916,17 +1915,24 @@ dummy_func(
DECREF_INPUTS();
ERROR_IF(true, error);
}
+
int err = 0;
- for (int i = 0; i < oparg; i++) {
+ for (Py_ssize_t i = 0; i < oparg; i++) {
+ _PyStackRef value = values[i];
+ values[i] = PyStackRef_NULL;
if (err == 0) {
- err = PySet_Add(set_o,
PyStackRef_AsPyObjectBorrow(values[i]));
+ err = _PySet_AddTakeRef((PySetObject *)set_o,
PyStackRef_AsPyObjectSteal(value));
+ }
+ else {
+ PyStackRef_CLOSE(value);
}
}
- DECREF_INPUTS();
- if (err != 0) {
+ if (err) {
Py_DECREF(set_o);
ERROR_IF(true, error);
}
+
+ INPUTS_DEAD();
set = PyStackRef_FromPyObjectStealMortal(set_o);
}
diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
index 2bd009e37556fc..2942680a222fb2 100644
--- a/Python/executor_cases.c.h
+++ b/Python/executor_cases.c.h
@@ -1481,17 +1481,16 @@
v = stack_pointer[-1];
set = stack_pointer[-2 - (oparg-1)];
_PyFrame_SetStackPointer(frame, stack_pointer);
- int err = PySet_Add(PyStackRef_AsPyObjectBorrow(set),
- PyStackRef_AsPyObjectBorrow(v));
- stack_pointer = _PyFrame_GetStackPointer(frame);
- stack_pointer += -1;
- assert(WITHIN_STACK_BOUNDS());
- _PyFrame_SetStackPointer(frame, stack_pointer);
- PyStackRef_CLOSE(v);
+ int err = _PySet_AddTakeRef((PySetObject
*)PyStackRef_AsPyObjectBorrow(set),
+ PyStackRef_AsPyObjectSteal(v));
stack_pointer = _PyFrame_GetStackPointer(frame);
if (err) {
+ stack_pointer += -1;
+ assert(WITHIN_STACK_BOUNDS());
JUMP_TO_ERROR();
}
+ stack_pointer += -1;
+ assert(WITHIN_STACK_BOUNDS());
break;
}
@@ -2687,32 +2686,31 @@
JUMP_TO_ERROR();
}
int err = 0;
- for (int i = 0; i < oparg; i++) {
+ for (Py_ssize_t i = 0; i < oparg; i++) {
+ _PyStackRef value = values[i];
+ values[i] = PyStackRef_NULL;
if (err == 0) {
_PyFrame_SetStackPointer(frame, stack_pointer);
- err = PySet_Add(set_o,
PyStackRef_AsPyObjectBorrow(values[i]));
+ err = _PySet_AddTakeRef((PySetObject *)set_o,
PyStackRef_AsPyObjectSteal(value));
+ stack_pointer = _PyFrame_GetStackPointer(frame);
+ }
+ else {
+ _PyFrame_SetStackPointer(frame, stack_pointer);
+ PyStackRef_CLOSE(value);
stack_pointer = _PyFrame_GetStackPointer(frame);
}
}
- _PyFrame_SetStackPointer(frame, stack_pointer);
- _PyStackRef tmp;
- for (int _i = oparg; --_i >= 0;) {
- tmp = values[_i];
- values[_i] = PyStackRef_NULL;
- PyStackRef_CLOSE(tmp);
- }
- stack_pointer = _PyFrame_GetStackPointer(frame);
- stack_pointer += -oparg;
- assert(WITHIN_STACK_BOUNDS());
- if (err != 0) {
+ if (err) {
_PyFrame_SetStackPointer(frame, stack_pointer);
Py_DECREF(set_o);
stack_pointer = _PyFrame_GetStackPointer(frame);
+ stack_pointer += -oparg;
+ assert(WITHIN_STACK_BOUNDS());
JUMP_TO_ERROR();
}
set = PyStackRef_FromPyObjectStealMortal(set_o);
- stack_pointer[0] = set;
- stack_pointer += 1;
+ stack_pointer[-oparg] = set;
+ stack_pointer += 1 - oparg;
assert(WITHIN_STACK_BOUNDS());
break;
}
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index 9ce2b633d5e506..a2e6c474d29204 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -1110,32 +1110,31 @@
JUMP_TO_LABEL(error);
}
int err = 0;
- for (int i = 0; i < oparg; i++) {
+ for (Py_ssize_t i = 0; i < oparg; i++) {
+ _PyStackRef value = values[i];
+ values[i] = PyStackRef_NULL;
if (err == 0) {
_PyFrame_SetStackPointer(frame, stack_pointer);
- err = PySet_Add(set_o,
PyStackRef_AsPyObjectBorrow(values[i]));
+ err = _PySet_AddTakeRef((PySetObject *)set_o,
PyStackRef_AsPyObjectSteal(value));
+ stack_pointer = _PyFrame_GetStackPointer(frame);
+ }
+ else {
+ _PyFrame_SetStackPointer(frame, stack_pointer);
+ PyStackRef_CLOSE(value);
stack_pointer = _PyFrame_GetStackPointer(frame);
}
}
- _PyFrame_SetStackPointer(frame, stack_pointer);
- _PyStackRef tmp;
- for (int _i = oparg; --_i >= 0;) {
- tmp = values[_i];
- values[_i] = PyStackRef_NULL;
- PyStackRef_CLOSE(tmp);
- }
- stack_pointer = _PyFrame_GetStackPointer(frame);
- stack_pointer += -oparg;
- assert(WITHIN_STACK_BOUNDS());
- if (err != 0) {
+ if (err) {
_PyFrame_SetStackPointer(frame, stack_pointer);
Py_DECREF(set_o);
stack_pointer = _PyFrame_GetStackPointer(frame);
+ stack_pointer += -oparg;
+ assert(WITHIN_STACK_BOUNDS());
JUMP_TO_LABEL(error);
}
set = PyStackRef_FromPyObjectStealMortal(set_o);
- stack_pointer[0] = set;
- stack_pointer += 1;
+ stack_pointer[-oparg] = set;
+ stack_pointer += 1 - oparg;
assert(WITHIN_STACK_BOUNDS());
DISPATCH();
}
@@ -10641,17 +10640,14 @@
v = stack_pointer[-1];
set = stack_pointer[-2 - (oparg-1)];
_PyFrame_SetStackPointer(frame, stack_pointer);
- int err = PySet_Add(PyStackRef_AsPyObjectBorrow(set),
- PyStackRef_AsPyObjectBorrow(v));
- stack_pointer = _PyFrame_GetStackPointer(frame);
- stack_pointer += -1;
- assert(WITHIN_STACK_BOUNDS());
- _PyFrame_SetStackPointer(frame, stack_pointer);
- PyStackRef_CLOSE(v);
+ int err = _PySet_AddTakeRef((PySetObject
*)PyStackRef_AsPyObjectBorrow(set),
+ PyStackRef_AsPyObjectSteal(v));
stack_pointer = _PyFrame_GetStackPointer(frame);
if (err) {
- JUMP_TO_LABEL(error);
+ JUMP_TO_LABEL(pop_1_error);
}
+ stack_pointer += -1;
+ assert(WITHIN_STACK_BOUNDS());
DISPATCH();
}
_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com
