From 3323cf09c5a0400688b731ddacd6476d9559ee5b Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 16 Jun 2024 16:32:02 +0200 Subject: [PATCH 1/4] Make enum_iter thread safe --- Include/internal/pycore_critical_section.h | 3 +++ Lib/test/test_enumerate.py | 23 +++++++++++++++++++ Objects/enumobject.c | 26 +++++++++++++++++----- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_critical_section.h b/Include/internal/pycore_critical_section.h index 7bebcdb67c7169..c6bbfdb108c469 100644 --- a/Include/internal/pycore_critical_section.h +++ b/Include/internal/pycore_critical_section.h @@ -98,6 +98,8 @@ extern "C" { # define Py_END_CRITICAL_SECTION() \ _PyCriticalSection_End(&_cs); \ } +# define Py_EXIT_CRITICAL_SECTION() \ + _PyCriticalSection_End(&_cs); # define Py_BEGIN_CRITICAL_SECTION2(a, b) \ { \ @@ -155,6 +157,7 @@ extern "C" { # define Py_BEGIN_CRITICAL_SECTION_MUT(mut) # define Py_BEGIN_CRITICAL_SECTION(op) # define Py_END_CRITICAL_SECTION() +# define Py_EXIT_CRITICAL_SECTION() # define Py_BEGIN_CRITICAL_SECTION2(a, b) # define Py_END_CRITICAL_SECTION2() # define Py_BEGIN_CRITICAL_SECTION_SEQUENCE_FAST(original) diff --git a/Lib/test/test_enumerate.py b/Lib/test/test_enumerate.py index 5cb54cff9b76fd..3038f228ff4d73 100644 --- a/Lib/test/test_enumerate.py +++ b/Lib/test/test_enumerate.py @@ -3,6 +3,7 @@ import sys import pickle import gc +import threading from test import support @@ -292,5 +293,27 @@ def enum(self, iterable, start=sys.maxsize + 1): (sys.maxsize+3,'c')] +class EnumerateThreading(unittest.TestCase): + @staticmethod + def work(index, enum, start): + while True: + try: + value = next(enum) + except StopIteration: + break + else: + if value[0] + start != value[1]: + raise ValueError(f'enumerate returned pair {value}') + + def test_threading(self): + number_of_threads = 4 + n = 100 + start = sys.maxsize-10 + enum = enumerate(range(start, start+n)) + worker_threads = [threading.Thread(target=self.work, args=[ii, enum, start]) for ii in range(number_of_threads)] + _ = [t.start() for t in worker_threads] + _ = [t.join() for t in worker_threads] + + if __name__ == "__main__": unittest.main() diff --git a/Objects/enumobject.c b/Objects/enumobject.c index 556666779d8522..60cfae1099bb93 100644 --- a/Objects/enumobject.c +++ b/Objects/enumobject.c @@ -2,6 +2,7 @@ #include "Python.h" #include "pycore_call.h" // _PyObject_CallNoArgs() +#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() #include "pycore_long.h" // _PyLong_GetOne() #include "pycore_modsupport.h" // _PyArg_NoKwnames() #include "pycore_object.h" // _PyObject_GC_TRACK() @@ -228,23 +229,38 @@ enum_next(enumobject *en) PyObject *it = en->en_sit; PyObject *old_index; PyObject *old_item; + int reuse_result = 0; + + Py_BEGIN_CRITICAL_SECTION(en); next_item = (*Py_TYPE(it)->tp_iternext)(it); - if (next_item == NULL) + if (next_item == NULL) { + Py_EXIT_CRITICAL_SECTION(); return NULL; - - if (en->en_index == PY_SSIZE_T_MAX) - return enum_next_long(en, next_item); + } + if (en->en_index == PY_SSIZE_T_MAX) { + // we hold on to the lock for the entire call to enum_next_long + result = enum_next_long(en, next_item); + Py_EXIT_CRITICAL_SECTION(); + return result; + } next_index = PyLong_FromSsize_t(en->en_index); if (next_index == NULL) { Py_DECREF(next_item); + Py_EXIT_CRITICAL_SECTION(); return NULL; } en->en_index++; - if (Py_REFCNT(result) == 1) { + reuse_result = Py_REFCNT(result) == 1; + if (reuse_result) { + // increment within the critical section Py_INCREF(result); + } + Py_END_CRITICAL_SECTION(); + + if (reuse_result) { old_index = PyTuple_GET_ITEM(result, 0); old_item = PyTuple_GET_ITEM(result, 1); PyTuple_SET_ITEM(result, 0, next_index); From 98f72e61393f6812c86fe3d4177c1c8ecfdeb835 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 16 Jun 2024 16:38:33 +0200 Subject: [PATCH 2/4] pep8 --- Lib/test/test_enumerate.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_enumerate.py b/Lib/test/test_enumerate.py index 3038f228ff4d73..14cf0b820a4ad6 100644 --- a/Lib/test/test_enumerate.py +++ b/Lib/test/test_enumerate.py @@ -3,7 +3,7 @@ import sys import pickle import gc -import threading +from threading import Thread from test import support @@ -295,7 +295,7 @@ def enum(self, iterable, start=sys.maxsize + 1): class EnumerateThreading(unittest.TestCase): @staticmethod - def work(index, enum, start): + def work(enum, start): while True: try: value = next(enum) @@ -309,8 +309,10 @@ def test_threading(self): number_of_threads = 4 n = 100 start = sys.maxsize-10 - enum = enumerate(range(start, start+n)) - worker_threads = [threading.Thread(target=self.work, args=[ii, enum, start]) for ii in range(number_of_threads)] + enum = enumerate(range(start, start + n)) + worker_threads = [] + for ii in range(number_of_threads): + worker_threads.append(Thread(target=self.work, args=[enum, start])) _ = [t.start() for t in worker_threads] _ = [t.join() for t in worker_threads] From ba17086d51143f3e717968d01a7b69378dd13167 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 16 Jun 2024 17:41:08 +0200 Subject: [PATCH 3/4] fix tests on wasi --- Lib/test/test_enumerate.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Lib/test/test_enumerate.py b/Lib/test/test_enumerate.py index 14cf0b820a4ad6..c28efba4fef21a 100644 --- a/Lib/test/test_enumerate.py +++ b/Lib/test/test_enumerate.py @@ -6,6 +6,7 @@ from threading import Thread from test import support +from test.support import threading_helper class G: 'Sequence using __getitem__' @@ -305,6 +306,8 @@ def work(enum, start): if value[0] + start != value[1]: raise ValueError(f'enumerate returned pair {value}') + @threading_helper.reap_threads + @threading_helper.requires_working_threading() def test_threading(self): number_of_threads = 4 n = 100 From 07d4574604c040d7081a1b4d818dbf4468240ace Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 16 Jun 2024 19:21:10 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2024-06-16-19-21-07.gh-issue-120496.oP4xPi.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-06-16-19-21-07.gh-issue-120496.oP4xPi.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-06-16-19-21-07.gh-issue-120496.oP4xPi.rst b/Misc/NEWS.d/next/Core and Builtins/2024-06-16-19-21-07.gh-issue-120496.oP4xPi.rst new file mode 100644 index 00000000000000..9b0d8676c6bfa8 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-06-16-19-21-07.gh-issue-120496.oP4xPi.rst @@ -0,0 +1 @@ +Make :class:`enumerate` thread-safe.