Skip to content

Commit

Permalink
pythongh-117657: Fix TSAN list set failure (python#118260)
Browse files Browse the repository at this point in the history
* Fix TSAN list set failure

* Relaxed atomic is sufficient, add targetted test

* More list

* Remove atomic assign in list

* Fixup white space
  • Loading branch information
DinoV authored and SonicField committed May 8, 2024
1 parent abd0602 commit 15b7ea6
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 3 deletions.
4 changes: 4 additions & 0 deletions Include/internal/pycore_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ _PyList_AppendTakeRef(PyListObject *self, PyObject *newitem)
Py_ssize_t allocated = self->allocated;
assert((size_t)len + 1 < PY_SSIZE_T_MAX);
if (allocated > len) {
#ifdef Py_GIL_DISABLED
_Py_atomic_store_ptr_release(&self->ob_item[len], newitem);
#else
PyList_SET_ITEM(self, len, newitem);
#endif
Py_SET_SIZE(self, len + 1);
return 0;
}
Expand Down
80 changes: 80 additions & 0 deletions Lib/test/test_free_threading/test_list.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import unittest

from threading import Thread
from unittest import TestCase

from test.support import is_wasi


class C:
def __init__(self, v):
self.v = v


@unittest.skipIf(is_wasi, "WASI has no threads.")
class TestList(TestCase):
def test_racing_iter_append(self):

l = []
OBJECT_COUNT = 10000

def writer_func():
for i in range(OBJECT_COUNT):
l.append(C(i + OBJECT_COUNT))

def reader_func():
while True:
count = len(l)
for i, x in enumerate(l):
self.assertEqual(x.v, i + OBJECT_COUNT)
if count == OBJECT_COUNT:
break

writer = Thread(target=writer_func)
readers = []
for x in range(30):
reader = Thread(target=reader_func)
readers.append(reader)
reader.start()

writer.start()
writer.join()
for reader in readers:
reader.join()

def test_racing_iter_extend(self):
iters = [
lambda x: [x],
]
for iter_case in iters:
with self.subTest(iter=iter_case):
l = []
OBJECT_COUNT = 10000

def writer_func():
for i in range(OBJECT_COUNT):
l.extend(iter_case(C(i + OBJECT_COUNT)))

def reader_func():
while True:
count = len(l)
for i, x in enumerate(l):
self.assertEqual(x.v, i + OBJECT_COUNT)
if count == OBJECT_COUNT:
break

writer = Thread(target=writer_func)
readers = []
for x in range(30):
reader = Thread(target=reader_func)
readers.append(reader)
reader.start()

writer.start()
writer.join()
for reader in readers:
reader.join()


if __name__ == "__main__":
unittest.main()
9 changes: 6 additions & 3 deletions Objects/listobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ list_resize(PyListObject *self, Py_ssize_t newsize)
target_bytes = allocated * sizeof(PyObject*);
}
memcpy(array->ob_item, self->ob_item, target_bytes);
}
if (new_allocated > (size_t)allocated) {
memset(array->ob_item + allocated, 0, sizeof(PyObject *) * (new_allocated - allocated));
}
_Py_atomic_store_ptr_release(&self->ob_item, &array->ob_item);
self->allocated = new_allocated;
Expand Down Expand Up @@ -502,7 +505,7 @@ _PyList_AppendTakeRefListResize(PyListObject *self, PyObject *newitem)
Py_DECREF(newitem);
return -1;
}
PyList_SET_ITEM(self, len, newitem);
FT_ATOMIC_STORE_PTR_RELEASE(self->ob_item[len], newitem);
return 0;
}

Expand Down Expand Up @@ -1181,7 +1184,7 @@ list_extend_fast(PyListObject *self, PyObject *iterable)
PyObject **dest = self->ob_item + m;
for (Py_ssize_t i = 0; i < n; i++) {
PyObject *o = src[i];
dest[i] = Py_NewRef(o);
FT_ATOMIC_STORE_PTR_RELEASE(dest[i], Py_NewRef(o));
}
return 0;
}
Expand Down Expand Up @@ -1238,7 +1241,7 @@ list_extend_iter_lock_held(PyListObject *self, PyObject *iterable)

if (Py_SIZE(self) < self->allocated) {
Py_ssize_t len = Py_SIZE(self);
PyList_SET_ITEM(self, len, item); // steals item ref
FT_ATOMIC_STORE_PTR_RELEASE(self->ob_item[len], item); // steals item ref
Py_SET_SIZE(self, len + 1);
}
else {
Expand Down

0 comments on commit 15b7ea6

Please sign in to comment.