https://github.com/python/cpython/commit/396f8b0b98441344e1d3223a4075e5e342e0c2df
commit: 396f8b0b98441344e1d3223a4075e5e342e0c2df
branch: 3.13
author: Miss Islington (bot) <31488909+miss-isling...@users.noreply.github.com>
committer: colesbury <colesb...@gmail.com>
date: 2024-06-17T19:12:25Z
summary:
[3.13] gh-117657: Fix `__slots__` thread safety in free-threaded build
(GH-119368) (#120655)
Fix a race in `PyMember_GetOne` and `PyMember_SetOne` for `Py_T_OBJECT_EX`.
These functions implement `__slots__` accesses for Python objects.
(cherry picked from commit 362cd2680b45a36c3467b9721ff7fc0ceb338452)
Co-authored-by: Daniele Parmeggiani <8658291+dpd...@users.noreply.github.com>
files:
A Lib/test/test_free_threading/test_slots.py
M Lib/test/test_descr.py
M Python/structmember.c
M Tools/tsan/suppressions_free_threading.txt
diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
index c3f292467a6738..14bd87eb9c8d84 100644
--- a/Lib/test/test_descr.py
+++ b/Lib/test/test_descr.py
@@ -1314,7 +1314,7 @@ class X(object):
# Inherit from object on purpose to check some backwards compatibility
paths
class X(object):
__slots__ = "a"
- with self.assertRaisesRegex(AttributeError, "'X' object has no
attribute 'a'"):
+ with self.assertRaisesRegex(AttributeError,
"'test.test_descr.ClassPropertiesAndMethods.test_slots.<locals>.X' object has
no attribute 'a'"):
X().a
# Test string subclass in `__slots__`, see gh-98783
diff --git a/Lib/test/test_free_threading/test_slots.py
b/Lib/test/test_free_threading/test_slots.py
new file mode 100644
index 00000000000000..758f74f54d0b56
--- /dev/null
+++ b/Lib/test/test_free_threading/test_slots.py
@@ -0,0 +1,43 @@
+import threading
+from test.support import threading_helper
+from unittest import TestCase
+
+
+def run_in_threads(targets):
+ """Run `targets` in separate threads"""
+ threads = [
+ threading.Thread(target=target)
+ for target in targets
+ ]
+ for thread in threads:
+ thread.start()
+ for thread in threads:
+ thread.join()
+
+
+@threading_helper.requires_working_threading()
+class TestSlots(TestCase):
+
+ def test_object(self):
+ class Spam:
+ __slots__ = [
+ "eggs",
+ ]
+
+ def __init__(self, initial_value):
+ self.eggs = initial_value
+
+ spam = Spam(0)
+ iters = 20_000
+
+ def writer():
+ for _ in range(iters):
+ spam.eggs += 1
+
+ def reader():
+ for _ in range(iters):
+ eggs = spam.eggs
+ assert type(eggs) is int
+ assert 0 <= eggs <= iters
+
+ run_in_threads([writer, reader, reader, reader])
diff --git a/Python/structmember.c b/Python/structmember.c
index ba881d18a0973d..d5e7ab83093dc8 100644
--- a/Python/structmember.c
+++ b/Python/structmember.c
@@ -4,8 +4,22 @@
#include "Python.h"
#include "pycore_abstract.h" // _PyNumber_Index()
#include "pycore_long.h" // _PyLong_IsNegative()
+#include "pycore_object.h" // _Py_TryIncrefCompare(), FT_ATOMIC_*()
+#include "pycore_critical_section.h"
+static inline PyObject *
+member_get_object(const char *addr, const char *obj_addr, PyMemberDef *l)
+{
+ PyObject *v = FT_ATOMIC_LOAD_PTR(*(PyObject **) addr);
+ if (v == NULL) {
+ PyErr_Format(PyExc_AttributeError,
+ "'%T' object has no attribute '%s'",
+ (PyObject *)obj_addr, l->name);
+ }
+ return v;
+}
+
PyObject *
PyMember_GetOne(const char *obj_addr, PyMemberDef *l)
{
@@ -75,15 +89,19 @@ PyMember_GetOne(const char *obj_addr, PyMemberDef *l)
Py_INCREF(v);
break;
case Py_T_OBJECT_EX:
- v = *(PyObject **)addr;
- if (v == NULL) {
- PyObject *obj = (PyObject *)obj_addr;
- PyTypeObject *tp = Py_TYPE(obj);
- PyErr_Format(PyExc_AttributeError,
- "'%.200s' object has no attribute '%s'",
- tp->tp_name, l->name);
- }
+ v = member_get_object(addr, obj_addr, l);
+#ifndef Py_GIL_DISABLED
Py_XINCREF(v);
+#else
+ if (v != NULL) {
+ if (!_Py_TryIncrefCompare((PyObject **) addr, v)) {
+ Py_BEGIN_CRITICAL_SECTION((PyObject *) obj_addr);
+ v = member_get_object(addr, obj_addr, l);
+ Py_XINCREF(v);
+ Py_END_CRITICAL_SECTION();
+ }
+ }
+#endif
break;
case Py_T_LONGLONG:
v = PyLong_FromLongLong(*(long long *)addr);
@@ -92,6 +110,7 @@ PyMember_GetOne(const char *obj_addr, PyMemberDef *l)
v = PyLong_FromUnsignedLongLong(*(unsigned long long *)addr);
break;
case _Py_T_NONE:
+ // doesn't require free-threading code path
v = Py_NewRef(Py_None);
break;
default:
@@ -118,6 +137,9 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
return -1;
}
+#ifdef Py_GIL_DISABLED
+ PyObject *obj = (PyObject *) addr;
+#endif
addr += l->offset;
if ((l->flags & Py_READONLY))
@@ -281,8 +303,10 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
break;
case _Py_T_OBJECT:
case Py_T_OBJECT_EX:
+ Py_BEGIN_CRITICAL_SECTION(obj);
oldv = *(PyObject **)addr;
- *(PyObject **)addr = Py_XNewRef(v);
+ FT_ATOMIC_STORE_PTR_RELEASE(*(PyObject **)addr, Py_XNewRef(v));
+ Py_END_CRITICAL_SECTION();
Py_XDECREF(oldv);
break;
case Py_T_CHAR: {
diff --git a/Tools/tsan/suppressions_free_threading.txt
b/Tools/tsan/suppressions_free_threading.txt
index b86c8fce11f57e..2986efe6774157 100644
--- a/Tools/tsan/suppressions_free_threading.txt
+++ b/Tools/tsan/suppressions_free_threading.txt
@@ -29,8 +29,6 @@ race_top:_PyEval_EvalFrameDefault
race_top:assign_version_tag
race_top:insertdict
race_top:lookup_tp_dict
-race_top:PyMember_GetOne
-race_top:PyMember_SetOne
race_top:new_reference
race_top:set_contains_key
# https://gist.github.com/colesbury/d13d033f413b4ad07929d044bed86c35
_______________________________________________
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
