Mirror
/
cpython
mirror of https://github.com/python/cpython
4
1
Fork 0
Code Issues Releases Wiki Activity

[3.13] gh-117657: Fix data races reported by TSAN in some set methods (GH-120914) (#121240) 

Refactor the fast Unicode hash check into `_PyObject_HashFast` and use relaxed
atomic loads in the free-threaded build.

After this change, the TSAN doesn't report data races for this method.
(cherry picked from commit 294e724964)

Co-authored-by: AN Long <aisk@users.noreply.github.com>
This commit is contained in:
Miss Islington (bot) 2024-07-01 21:40:28 +02:00 committed by GitHub
parent fc0b1cbdfe
commit 06fd745dd9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 81 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
Include/internal/pycore_object.h
View File

@ -628,6 +628,20 @@ _PyObject_IS_GC(PyObject *obj)
&& (type->tp_is_gc == NULL || type->tp_is_gc(obj)));
}
// Fast inlined version of PyObject_Hash()
static inline Py_hash_t
_PyObject_HashFast(PyObject *op)
{
if (PyUnicode_CheckExact(op)) {
Py_hash_t hash = FT_ATOMIC_LOAD_SSIZE_RELAXED(
_PyASCIIObject_CAST(op)->hash);
if (hash != -1) {
return hash;
}
}
return PyObject_Hash(op);
}
// Fast inlined version of PyType_IS_GC()
#define _PyType_IS_GC(t) _PyType_HasFeature((t), Py_TPFLAGS_HAVE_GC)

7
Modules/_collectionsmodule.c
View File

@ -2537,11 +2537,8 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping,
if (key == NULL)
break;
if (!PyUnicode_CheckExact(key) ||
(hash = _PyASCIIObject_CAST(key)->hash) == -1)
{
hash = PyObject_Hash(key);
if (hash == -1)
hash = _PyObject_HashFast(key);
if (hash == -1) {
goto done;
}

75
Objects/dictobject.c
View File

@ -427,7 +427,7 @@ static inline Py_hash_t
unicode_get_hash(PyObject *o)
{
assert(PyUnicode_CheckExact(o));
return _PyASCIIObject_CAST(o)->hash;
return FT_ATOMIC_LOAD_SSIZE_RELAXED(_PyASCIIObject_CAST(o)->hash);
}
/* Print summary info about the state of the optimized allocator */
@ -2170,14 +2170,11 @@ dict_getitem(PyObject *op, PyObject *key, const char *warnmsg)
}
PyDictObject *mp = (PyDictObject *)op;
Py_hash_t hash;
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
hash = PyObject_Hash(key);
Py_hash_t hash = _PyObject_HashFast(key);
if (hash == -1) {
PyErr_FormatUnraisable(warnmsg);
return NULL;
}
}
PyThreadState *tstate = _PyThreadState_GET();
#ifdef Py_DEBUG
@ -2225,13 +2222,10 @@ _PyDict_LookupIndex(PyDictObject *mp, PyObject *key)
assert(PyDict_CheckExact((PyObject*)mp));
assert(PyUnicode_CheckExact(key));
Py_hash_t hash = unicode_get_hash(key);
if (hash == -1) {
hash = PyObject_Hash(key);
Py_hash_t hash = _PyObject_HashFast(key);
if (hash == -1) {
return -1;
}
}
return _Py_dict_lookup(mp, key, hash, &value);
}
@ -2301,15 +2295,11 @@ PyDict_GetItemRef(PyObject *op, PyObject *key, PyObject **result)
return -1;
}
Py_hash_t hash;
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1)
{
hash = PyObject_Hash(key);
Py_hash_t hash = _PyObject_HashFast(key);
if (hash == -1) {
*result = NULL;
return -1;
}
}
return _PyDict_GetItemRef_KnownHash((PyDictObject *)op, key, hash, result);
}
@ -2320,14 +2310,11 @@ _PyDict_GetItemRef_Unicode_LockHeld(PyDictObject *op, PyObject *key, PyObject **
ASSERT_DICT_LOCKED(op);
assert(PyUnicode_CheckExact(key));
Py_hash_t hash;
if ((hash = unicode_get_hash(key)) == -1) {
hash = PyObject_Hash(key);
Py_hash_t hash = _PyObject_HashFast(key);
if (hash == -1) {
*result = NULL;
return -1;
}
}
PyObject *value;
Py_ssize_t ix = _Py_dict_lookup(op, key, hash, &value);
@ -2360,13 +2347,10 @@ PyDict_GetItemWithError(PyObject *op, PyObject *key)
PyErr_BadInternalCall();
return NULL;
}
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1)
{
hash = PyObject_Hash(key);
hash = _PyObject_HashFast(key);
if (hash == -1) {
return NULL;
}
}
#ifdef Py_GIL_DISABLED
ix = _Py_dict_lookup_threadsafe(mp, key, hash, &value);
@ -2433,9 +2417,8 @@ _PyDict_LoadGlobal(PyDictObject *globals, PyDictObject *builtins, PyObject *key)
Py_hash_t hash;
PyObject *value;
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
hash = PyObject_Hash(key);
if (hash == -1)
hash = _PyObject_HashFast(key);
if (hash == -1) {
return NULL;
}
@ -2461,15 +2444,12 @@ setitem_take2_lock_held(PyDictObject *mp, PyObject *key, PyObject *value)
assert(key);
assert(value);
assert(PyDict_Check(mp));
Py_hash_t hash;
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
hash = PyObject_Hash(key);
Py_hash_t hash = _PyObject_HashFast(key);
if (hash == -1) {
Py_DECREF(key);
Py_DECREF(value);
return -1;
}
}
PyInterpreterState *interp = _PyInterpreterState_GET();
@ -2617,11 +2597,9 @@ delitem_common(PyDictObject *mp, Py_hash_t hash, Py_ssize_t ix,
int
PyDict_DelItem(PyObject *op, PyObject *key)
{
Py_hash_t hash;
assert(key);
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
hash = PyObject_Hash(key);
if (hash == -1)
Py_hash_t hash = _PyObject_HashFast(key);
if (hash == -1) {
return -1;
}
@ -2946,16 +2924,13 @@ pop_lock_held(PyObject *op, PyObject *key, PyObject **result)
return 0;
}
Py_hash_t hash;
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
hash = PyObject_Hash(key);
Py_hash_t hash = _PyObject_HashFast(key);
if (hash == -1) {
if (result) {
*result = NULL;
}
return -1;
}
}
return _PyDict_Pop_KnownHash(dict, key, hash, result);
}
@ -3285,9 +3260,8 @@ dict_subscript(PyObject *self, PyObject *key)
Py_hash_t hash;
PyObject *value;
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
hash = PyObject_Hash(key);
if (hash == -1)
hash = _PyObject_HashFast(key);
if (hash == -1) {
return NULL;
}
ix = _Py_dict_lookup_threadsafe(mp, key, hash, &value);
@ -4172,9 +4146,8 @@ dict_get_impl(PyDictObject *self, PyObject *key, PyObject *default_value)
Py_hash_t hash;
Py_ssize_t ix;
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
hash = PyObject_Hash(key);
if (hash == -1)
hash = _PyObject_HashFast(key);
if (hash == -1) {
return NULL;
}
ix = _Py_dict_lookup_threadsafe(self, key, hash, &val);
@ -4205,15 +4178,13 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
return -1;
}
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
hash = PyObject_Hash(key);
hash = _PyObject_HashFast(key);
if (hash == -1) {
if (result) {
*result = NULL;
}
return -1;
}
}
if (mp->ma_keys == Py_EMPTY_KEYS) {
if (insert_to_emptydict(interp, mp, Py_NewRef(key), hash,
@ -4642,11 +4613,9 @@ static PyMethodDef mapp_methods[] = {
int
PyDict_Contains(PyObject *op, PyObject *key)
{
Py_hash_t hash;
Py_hash_t hash = _PyObject_HashFast(key);
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
hash = PyObject_Hash(key);
if (hash == -1)
if (hash == -1) {
return -1;
}
@ -6727,10 +6696,8 @@ int
_PyDict_SetItem_LockHeld(PyDictObject *dict, PyObject *name, PyObject *value)
{
if (value == NULL) {
Py_hash_t hash;
if (!PyUnicode_CheckExact(name) || (hash = unicode_get_hash(name)) == -1) {
hash = PyObject_Hash(name);
if (hash == -1)
Py_hash_t hash = _PyObject_HashFast(name);
if (hash == -1) {
return -1;
}
return delitem_knownhash_lock_held((PyObject *)dict, name, hash);

24
Objects/setobject.c
View File

@ -365,12 +365,8 @@ set_discard_entry(PySetObject *so, PyObject *key, Py_hash_t hash)
static int
set_add_key(PySetObject *so, PyObject *key)
{
Py_hash_t hash;
if (!PyUnicode_CheckExact(key) ||
(hash = _PyASCIIObject_CAST(key)->hash) == -1) {
hash = PyObject_Hash(key);
if (hash == -1)
Py_hash_t hash = _PyObject_HashFast(key);
if (hash == -1) {
return -1;
}
return set_add_entry(so, key, hash);
@ -379,12 +375,8 @@ set_add_key(PySetObject *so, PyObject *key)
static int
set_contains_key(PySetObject *so, PyObject *key)
{
Py_hash_t hash;
if (!PyUnicode_CheckExact(key) ||
(hash = _PyASCIIObject_CAST(key)->hash) == -1) {
hash = PyObject_Hash(key);
if (hash == -1)
Py_hash_t hash = _PyObject_HashFast(key);
if (hash == -1) {
return -1;
}
return set_contains_entry(so, key, hash);
@ -393,12 +385,8 @@ set_contains_key(PySetObject *so, PyObject *key)
static int
set_discard_key(PySetObject *so, PyObject *key)
{
Py_hash_t hash;
if (!PyUnicode_CheckExact(key) ||
(hash = _PyASCIIObject_CAST(key)->hash) == -1) {
hash = PyObject_Hash(key);
if (hash == -1)
Py_hash_t hash = _PyObject_HashFast(key);
if (hash == -1) {
return -1;
}
return set_discard_entry(so, key, hash);

7
Objects/typeobject.c
View File

@ -5069,16 +5069,11 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error)
{
ASSERT_TYPE_LOCK_HELD();
Py_hash_t hash;
if (!PyUnicode_CheckExact(name) ||
(hash = _PyASCIIObject_CAST(name)->hash) == -1)
{
hash = PyObject_Hash(name);
Py_hash_t hash = _PyObject_HashFast(name);
if (hash == -1) {
*error = -1;
return NULL;
}
}
/* Look in tp_dict of types in MRO */
PyObject *mro = lookup_tp_mro(type);

1
Tools/tsan/suppressions_free_threading.txt
View File

@ -30,7 +30,6 @@ race_top:assign_version_tag
race_top:insertdict
race_top:lookup_tp_dict
race_top:new_reference
race_top:set_contains_key
# https://gist.github.com/colesbury/d13d033f413b4ad07929d044bed86c35
race_top:set_discard_entry
race_top:_PyDict_CheckConsistency