From eeba1c28e89a0a16a82d58186a4f39852a1d3f06 Mon Sep 17 00:00:00 2001 From: AN Long Date: Sat, 27 Apr 2024 17:25:19 +0800 Subject: [PATCH 01/15] Make the filed_limit thread safe --- Modules/_csv.c | 16 +++++++++++----- Modules/clinic/_csv.c.h | 5 ++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index ac948f417cebf5..c4021ab30df866 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -34,7 +34,7 @@ typedef struct { PyTypeObject *dialect_type; PyTypeObject *reader_type; PyTypeObject *writer_type; - long field_limit; /* max parsed field size */ + int32_t field_limit; /* max parsed field size */ PyObject *str_write; } _csvstate; @@ -702,10 +702,15 @@ parse_grow_buff(ReaderObj *self) static int parse_add_char(ReaderObj *self, _csvstate *module_state, Py_UCS4 c) { - if (self->field_len >= module_state->field_limit) { +#ifdef Py_GIL_DISABLED + uint32_t field_limit = _Py_atomic_load_int32(&module_state->field_limit); +#else + uint32_t field_limit = module_state->field_limit; +#endif + if (self->field_len >= field_limit) { PyErr_Format(module_state->error_obj, "field larger than field limit (%ld)", - module_state->field_limit); + field_limit); return -1; } if (self->field_len == self->field_size && !parse_grow_buff(self)) @@ -1635,6 +1640,7 @@ _csv_get_dialect_impl(PyObject *module, PyObject *name) } /*[clinic input] +@critical_section _csv.field_size_limit new_limit: object = NULL @@ -1649,10 +1655,10 @@ the old limit is returned static PyObject * _csv_field_size_limit_impl(PyObject *module, PyObject *new_limit) -/*[clinic end generated code: output=f2799ecd908e250b input=cec70e9226406435]*/ +/*[clinic end generated code: output=f2799ecd908e250b input=3e49d42e37a7d449]*/ { _csvstate *module_state = get_csv_state(module); - long old_limit = module_state->field_limit; + int32_t old_limit = module_state->field_limit; if (new_limit != NULL) { if (!PyLong_CheckExact(new_limit)) { PyErr_Format(PyExc_TypeError, diff --git a/Modules/clinic/_csv.c.h b/Modules/clinic/_csv.c.h index 2442bdcde5679f..0c9fc32e50e993 100644 --- a/Modules/clinic/_csv.c.h +++ b/Modules/clinic/_csv.c.h @@ -6,6 +6,7 @@ preserve # include "pycore_gc.h" // PyGC_Head # include "pycore_runtime.h" // _Py_ID() #endif +#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() #include "pycore_modsupport.h" // _PyArg_UnpackKeywords() PyDoc_STRVAR(_csv_list_dialects__doc__, @@ -201,9 +202,11 @@ _csv_field_size_limit(PyObject *module, PyObject *const *args, Py_ssize_t nargs, } new_limit = args[0]; skip_optional_pos: + Py_BEGIN_CRITICAL_SECTION(module); return_value = _csv_field_size_limit_impl(module, new_limit); + Py_END_CRITICAL_SECTION(); exit: return return_value; } -/*[clinic end generated code: output=9ec59717f5414d8b input=a9049054013a1b77]*/ +/*[clinic end generated code: output=c9376d6674176385 input=a9049054013a1b77]*/ From 40e71a8717d3cf5418b329f77a8cf36a26c9e3d0 Mon Sep 17 00:00:00 2001 From: AN Long Date: Sat, 27 Apr 2024 18:16:40 +0800 Subject: [PATCH 02/15] Make dialects fields on module state thread safe --- Modules/_csv.c | 9 +++++++-- Modules/clinic/_csv.c.h | 4 +++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index c4021ab30df866..e1ecb6007f544d 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -1576,6 +1576,7 @@ csv_register_dialect(PyObject *module, PyObject *args, PyObject *kwargs) PyObject *name_obj, *dialect_obj = NULL; _csvstate *module_state = get_csv_state(module); PyObject *dialect; + int res; if (!PyArg_UnpackTuple(args, "", 1, 2, &name_obj, &dialect_obj)) return NULL; @@ -1587,7 +1588,10 @@ csv_register_dialect(PyObject *module, PyObject *args, PyObject *kwargs) dialect = _call_dialect(module_state, dialect_obj, kwargs); if (dialect == NULL) return NULL; - if (PyDict_SetItem(module_state->dialects, name_obj, dialect) < 0) { + Py_BEGIN_CRITICAL_SECTION(module_state->dialects); + res = PyDict_SetItem(module_state->dialects, name_obj, dialect); + Py_END_CRITICAL_SECTION(); + if (res < 0) { Py_DECREF(dialect); return NULL; } @@ -1597,6 +1601,7 @@ csv_register_dialect(PyObject *module, PyObject *args, PyObject *kwargs) /*[clinic input] +@critical_section _csv.unregister_dialect name: object @@ -1608,7 +1613,7 @@ Delete the name/dialect mapping associated with a string name. static PyObject * _csv_unregister_dialect_impl(PyObject *module, PyObject *name) -/*[clinic end generated code: output=0813ebca6c058df4 input=6b5c1557bf60c7e7]*/ +/*[clinic end generated code: output=0813ebca6c058df4 input=c38732b506218713]*/ { _csvstate *module_state = get_csv_state(module); int rc = PyDict_Pop(module_state->dialects, name, NULL); diff --git a/Modules/clinic/_csv.c.h b/Modules/clinic/_csv.c.h index 0c9fc32e50e993..4746aa9747a44c 100644 --- a/Modules/clinic/_csv.c.h +++ b/Modules/clinic/_csv.c.h @@ -80,7 +80,9 @@ _csv_unregister_dialect(PyObject *module, PyObject *const *args, Py_ssize_t narg goto exit; } name = args[0]; + Py_BEGIN_CRITICAL_SECTION(module); return_value = _csv_unregister_dialect_impl(module, name); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -209,4 +211,4 @@ _csv_field_size_limit(PyObject *module, PyObject *const *args, Py_ssize_t nargs, exit: return return_value; } -/*[clinic end generated code: output=c9376d6674176385 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=4d7e42aba40bd6c2 input=a9049054013a1b77]*/ From c77750fb15bbaa93b478b42055ffc431837090d2 Mon Sep 17 00:00:00 2001 From: AN Long Date: Sat, 27 Apr 2024 20:00:46 +0800 Subject: [PATCH 03/15] Utilize FT_ATOMIC_LOAD_INT32 --- Include/internal/pycore_pyatomic_ft_wrappers.h | 2 ++ Modules/_csv.c | 7 ++----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index fed5d6e0ec2c54..4ea9b2d262657b 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -40,6 +40,7 @@ extern "C" { _Py_atomic_store_ssize_relaxed(&value, new_value) #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \ _Py_atomic_store_uint8_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_INT32(value) _Py_atomic_load_int32(&value) #else #define FT_ATOMIC_LOAD_PTR(value) value @@ -53,6 +54,7 @@ extern "C" { #define FT_ATOMIC_STORE_UINTPTR_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_INT32(value) value #endif diff --git a/Modules/_csv.c b/Modules/_csv.c index e1ecb6007f544d..16328e2a60723e 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -14,6 +14,7 @@ module instead. #endif #include "Python.h" +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_INT32 #include // offsetof() #include @@ -702,11 +703,7 @@ parse_grow_buff(ReaderObj *self) static int parse_add_char(ReaderObj *self, _csvstate *module_state, Py_UCS4 c) { -#ifdef Py_GIL_DISABLED - uint32_t field_limit = _Py_atomic_load_int32(&module_state->field_limit); -#else - uint32_t field_limit = module_state->field_limit; -#endif + uint32_t field_limit = FT_ATOMIC_LOAD_INT32(module_state->field_limit); if (self->field_len >= field_limit) { PyErr_Format(module_state->error_obj, "field larger than field limit (%ld)", From 6fbcb9f5638c4b73e18a699156f87d05d14a5cf7 Mon Sep 17 00:00:00 2001 From: AN Long Date: Sat, 27 Apr 2024 22:32:00 +0800 Subject: [PATCH 04/15] Remove critical section on `dialects` field --- Modules/_csv.c | 5 +---- Modules/clinic/_csv.c.h | 4 +--- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index 16328e2a60723e..7b3101732945e3 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -1585,9 +1585,7 @@ csv_register_dialect(PyObject *module, PyObject *args, PyObject *kwargs) dialect = _call_dialect(module_state, dialect_obj, kwargs); if (dialect == NULL) return NULL; - Py_BEGIN_CRITICAL_SECTION(module_state->dialects); res = PyDict_SetItem(module_state->dialects, name_obj, dialect); - Py_END_CRITICAL_SECTION(); if (res < 0) { Py_DECREF(dialect); return NULL; @@ -1598,7 +1596,6 @@ csv_register_dialect(PyObject *module, PyObject *args, PyObject *kwargs) /*[clinic input] -@critical_section _csv.unregister_dialect name: object @@ -1610,7 +1607,7 @@ Delete the name/dialect mapping associated with a string name. static PyObject * _csv_unregister_dialect_impl(PyObject *module, PyObject *name) -/*[clinic end generated code: output=0813ebca6c058df4 input=c38732b506218713]*/ +/*[clinic end generated code: output=0813ebca6c058df4 input=6b5c1557bf60c7e7]*/ { _csvstate *module_state = get_csv_state(module); int rc = PyDict_Pop(module_state->dialects, name, NULL); diff --git a/Modules/clinic/_csv.c.h b/Modules/clinic/_csv.c.h index 4746aa9747a44c..0c9fc32e50e993 100644 --- a/Modules/clinic/_csv.c.h +++ b/Modules/clinic/_csv.c.h @@ -80,9 +80,7 @@ _csv_unregister_dialect(PyObject *module, PyObject *const *args, Py_ssize_t narg goto exit; } name = args[0]; - Py_BEGIN_CRITICAL_SECTION(module); return_value = _csv_unregister_dialect_impl(module, name); - Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -211,4 +209,4 @@ _csv_field_size_limit(PyObject *module, PyObject *const *args, Py_ssize_t nargs, exit: return return_value; } -/*[clinic end generated code: output=4d7e42aba40bd6c2 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=c9376d6674176385 input=a9049054013a1b77]*/ From 60753f1d42dceecb1249167ace979ef0ec586dd5 Mon Sep 17 00:00:00 2001 From: AN Long Date: Mon, 29 Apr 2024 23:06:43 +0800 Subject: [PATCH 05/15] Totally revert the change to csv_register_dialect --- Modules/_csv.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index 7b3101732945e3..d060238ccef368 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -1573,7 +1573,6 @@ csv_register_dialect(PyObject *module, PyObject *args, PyObject *kwargs) PyObject *name_obj, *dialect_obj = NULL; _csvstate *module_state = get_csv_state(module); PyObject *dialect; - int res; if (!PyArg_UnpackTuple(args, "", 1, 2, &name_obj, &dialect_obj)) return NULL; @@ -1585,8 +1584,7 @@ csv_register_dialect(PyObject *module, PyObject *args, PyObject *kwargs) dialect = _call_dialect(module_state, dialect_obj, kwargs); if (dialect == NULL) return NULL; - res = PyDict_SetItem(module_state->dialects, name_obj, dialect); - if (res < 0) { + if (PyDict_SetItem(module_state->dialects, name_obj, dialect) < 0) { Py_DECREF(dialect); return NULL; } From 64b7d209bf8bb05c17739793770b256f4330ef20 Mon Sep 17 00:00:00 2001 From: AN Long Date: Mon, 29 Apr 2024 23:34:50 +0800 Subject: [PATCH 06/15] Revert field_limit to long, and add atomic api for it --- Include/cpython/pyatomic_gcc.h | 4 ++++ Include/internal/pycore_pyatomic_ft_wrappers.h | 4 ++-- Modules/_csv.c | 6 +++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Include/cpython/pyatomic_gcc.h b/Include/cpython/pyatomic_gcc.h index af78a94c736545..6b0497a86cc2df 100644 --- a/Include/cpython/pyatomic_gcc.h +++ b/Include/cpython/pyatomic_gcc.h @@ -247,6 +247,10 @@ static inline int _Py_atomic_load_int(const int *obj) { return __atomic_load_n(obj, __ATOMIC_SEQ_CST); } +static inline long +_Py_atomic_load_long(const long *obj) +{ return __atomic_load_n(obj, __ATOMIC_SEQ_CST); } + static inline int8_t _Py_atomic_load_int8(const int8_t *obj) { return __atomic_load_n(obj, __ATOMIC_SEQ_CST); } diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index 4ea9b2d262657b..018fae099c2fcd 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -40,7 +40,7 @@ extern "C" { _Py_atomic_store_ssize_relaxed(&value, new_value) #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \ _Py_atomic_store_uint8_relaxed(&value, new_value) -#define FT_ATOMIC_LOAD_INT32(value) _Py_atomic_load_int32(&value) +#define FT_ATOMIC_LOAD_LONG(value) _Py_atomic_load_long(&value) #else #define FT_ATOMIC_LOAD_PTR(value) value @@ -54,7 +54,7 @@ extern "C" { #define FT_ATOMIC_STORE_UINTPTR_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value -#define FT_ATOMIC_LOAD_INT32(value) value +#define FT_ATOMIC_LOAD_LONG(value) value #endif diff --git a/Modules/_csv.c b/Modules/_csv.c index d060238ccef368..1723ca21ec3dba 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -14,7 +14,7 @@ module instead. #endif #include "Python.h" -#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_INT32 +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_LONG #include // offsetof() #include @@ -35,7 +35,7 @@ typedef struct { PyTypeObject *dialect_type; PyTypeObject *reader_type; PyTypeObject *writer_type; - int32_t field_limit; /* max parsed field size */ + long field_limit; /* max parsed field size */ PyObject *str_write; } _csvstate; @@ -703,7 +703,7 @@ parse_grow_buff(ReaderObj *self) static int parse_add_char(ReaderObj *self, _csvstate *module_state, Py_UCS4 c) { - uint32_t field_limit = FT_ATOMIC_LOAD_INT32(module_state->field_limit); + long field_limit = FT_ATOMIC_LOAD_LONG(module_state->field_limit); if (self->field_len >= field_limit) { PyErr_Format(module_state->error_obj, "field larger than field limit (%ld)", From 1de333e5bb7143a64d51736e30a0871733a2d572 Mon Sep 17 00:00:00 2001 From: AN Long Date: Mon, 29 Apr 2024 23:50:28 +0800 Subject: [PATCH 07/15] Add _Py_atomic_load_long for other platforms --- Include/cpython/pyatomic.h | 3 +++ Include/cpython/pyatomic_msc.h | 7 +++++++ Include/cpython/pyatomic_std.h | 7 +++++++ 3 files changed, 17 insertions(+) diff --git a/Include/cpython/pyatomic.h b/Include/cpython/pyatomic.h index 69083f1d9dd0c2..865cd5b89e9b55 100644 --- a/Include/cpython/pyatomic.h +++ b/Include/cpython/pyatomic.h @@ -275,6 +275,9 @@ _Py_atomic_or_uintptr(uintptr_t *obj, uintptr_t value); static inline int _Py_atomic_load_int(const int *obj); +static inline long +_Py_atomic_load_long(const long *obj); + static inline int8_t _Py_atomic_load_int8(const int8_t *obj); diff --git a/Include/cpython/pyatomic_msc.h b/Include/cpython/pyatomic_msc.h index 212cd7817d01c5..c172406d1b76cd 100644 --- a/Include/cpython/pyatomic_msc.h +++ b/Include/cpython/pyatomic_msc.h @@ -581,6 +581,13 @@ _Py_atomic_load_int(const int *obj) return (int)_Py_atomic_load_uint32((uint32_t *)obj); } +static inline long +_Py_atomic_load_long(const long *obj) +{ + _Py_atomic_ASSERT_ARG_TYPE(uint32_t); + return (int)_Py_atomic_load_uint32((uint32_t *)obj); +} + static inline unsigned int _Py_atomic_load_uint(const unsigned int *obj) { diff --git a/Include/cpython/pyatomic_std.h b/Include/cpython/pyatomic_std.h index 6a77eae536d8dd..f2c5158da9ba9f 100644 --- a/Include/cpython/pyatomic_std.h +++ b/Include/cpython/pyatomic_std.h @@ -413,6 +413,13 @@ _Py_atomic_load_int(const int *obj) return atomic_load((const _Atomic(int)*)obj); } +static inline long +_Py_atomic_load_long(const long *obj) +{ + _Py_USING_STD; + return atomic_load((const _Atomic(long)*)obj); +} + static inline int8_t _Py_atomic_load_int8(const int8_t *obj) { From 0a41927c42d9917afc3717d14d6073858d25a3cf Mon Sep 17 00:00:00 2001 From: AN Long Date: Wed, 15 May 2024 22:49:10 +0800 Subject: [PATCH 08/15] Revert the type change to old_limit --- Modules/_csv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index de6534a288abac..064ef9f6966369 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -1655,7 +1655,7 @@ _csv_field_size_limit_impl(PyObject *module, PyObject *new_limit) /*[clinic end generated code: output=f2799ecd908e250b input=3e49d42e37a7d449]*/ { _csvstate *module_state = get_csv_state(module); - int32_t old_limit = module_state->field_limit; + long old_limit = module_state->field_limit; if (new_limit != NULL) { if (!PyLong_CheckExact(new_limit)) { PyErr_Format(PyExc_TypeError, From f4130921e61e957d198e5dbd66fa060fbec518b5 Mon Sep 17 00:00:00 2001 From: AN Long Date: Thu, 16 May 2024 00:28:39 +0800 Subject: [PATCH 09/15] Using atomic_store instead of critical_section --- Include/cpython/pyatomic.h | 3 +++ Include/cpython/pyatomic_gcc.h | 4 ++++ Include/cpython/pyatomic_msc.h | 14 ++++++++++++++ Include/cpython/pyatomic_std.h | 7 +++++++ Include/internal/pycore_pyatomic_ft_wrappers.h | 3 +++ Modules/_csv.c | 9 ++++----- 6 files changed, 35 insertions(+), 5 deletions(-) diff --git a/Include/cpython/pyatomic.h b/Include/cpython/pyatomic.h index 4556ae05927f2b..0e69691d1a9c87 100644 --- a/Include/cpython/pyatomic.h +++ b/Include/cpython/pyatomic.h @@ -372,6 +372,9 @@ _Py_atomic_load_ullong_relaxed(const unsigned long long *obj); static inline void _Py_atomic_store_int(int *obj, int value); +static inline void +_Py_atomic_store_long(long *obj, long value); + static inline void _Py_atomic_store_int8(int8_t *obj, int8_t value); diff --git a/Include/cpython/pyatomic_gcc.h b/Include/cpython/pyatomic_gcc.h index 881696eba2abe5..e8da2c656877e4 100644 --- a/Include/cpython/pyatomic_gcc.h +++ b/Include/cpython/pyatomic_gcc.h @@ -373,6 +373,10 @@ static inline void _Py_atomic_store_int(int *obj, int value) { __atomic_store_n(obj, value, __ATOMIC_SEQ_CST); } +static inline void +_Py_atomic_store_long(long *obj, long value) +{ __atomic_store_n(obj, value, __ATOMIC_SEQ_CST); } + static inline void _Py_atomic_store_int8(int8_t *obj, int8_t value) { __atomic_store_n(obj, value, __ATOMIC_SEQ_CST); } diff --git a/Include/cpython/pyatomic_msc.h b/Include/cpython/pyatomic_msc.h index bcf7d9e8f24539..4d4c5a5f735fbf 100644 --- a/Include/cpython/pyatomic_msc.h +++ b/Include/cpython/pyatomic_msc.h @@ -355,6 +355,14 @@ _Py_atomic_exchange_int(int *obj, int value) (int32_t)value); } +static inline int +_Py_atomic_exchange_long(long *obj, long value) +{ + _Py_atomic_ASSERT_ARG_TYPE(int32_t); + return (int)_Py_atomic_exchange_int32((int32_t *)obj, + (int32_t)value); +} + static inline unsigned int _Py_atomic_exchange_uint(unsigned int *obj, unsigned int value) { @@ -734,6 +742,12 @@ _Py_atomic_store_int(int *obj, int value) (void)_Py_atomic_exchange_int(obj, value); } +static inline void +_Py_atomic_store_long(long *obj, long value) +{ + (void)_Py_atomic_exchange_long(obj, value); +} + static inline void _Py_atomic_store_int8(int8_t *obj, int8_t value) { diff --git a/Include/cpython/pyatomic_std.h b/Include/cpython/pyatomic_std.h index ecad14ace97c4d..23ffc210ad41ca 100644 --- a/Include/cpython/pyatomic_std.h +++ b/Include/cpython/pyatomic_std.h @@ -644,6 +644,13 @@ _Py_atomic_store_int(int *obj, int value) atomic_store((_Atomic(int)*)obj, value); } +static inline void +_Py_atomic_store_long(long *obj, long value) +{ + _Py_USING_STD; + atomic_store((_Atomic(long)*)obj, value); +} + static inline void _Py_atomic_store_int8(int8_t *obj, int8_t value) { diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index 75af14e9116d73..2fd1090dc66a16 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -56,6 +56,8 @@ extern "C" { #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \ _Py_atomic_store_uint8_relaxed(&value, new_value) #define FT_ATOMIC_LOAD_LONG(value) _Py_atomic_load_long(&value) +#define FT_ATOMIC_STORE_LONG(value, new_value) \ + _Py_atomic_store_long(&value, new_value) #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) \ _Py_atomic_store_uint16_relaxed(&value, new_value) #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) \ @@ -82,6 +84,7 @@ extern "C" { #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_LOAD_LONG(value) value +#define FT_ATOMIC_STORE_LONG(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) value = new_value diff --git a/Modules/_csv.c b/Modules/_csv.c index 064ef9f6966369..e6e3b39f36224e 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -1637,7 +1637,6 @@ _csv_get_dialect_impl(PyObject *module, PyObject *name) } /*[clinic input] -@critical_section _csv.field_size_limit new_limit: object = NULL @@ -1655,18 +1654,18 @@ _csv_field_size_limit_impl(PyObject *module, PyObject *new_limit) /*[clinic end generated code: output=f2799ecd908e250b input=3e49d42e37a7d449]*/ { _csvstate *module_state = get_csv_state(module); - long old_limit = module_state->field_limit; + long old_limit = FT_ATOMIC_LOAD_LONG(module_state->field_limit); if (new_limit != NULL) { if (!PyLong_CheckExact(new_limit)) { PyErr_Format(PyExc_TypeError, "limit must be an integer"); return NULL; } - module_state->field_limit = PyLong_AsLong(new_limit); - if (module_state->field_limit == -1 && PyErr_Occurred()) { - module_state->field_limit = old_limit; + long new_limit_value = PyLong_AsLong(new_limit); + if (new_limit_value == -1 && PyErr_Occurred()) { return NULL; } + FT_ATOMIC_STORE_LONG(module_state->field_limit, new_limit_value); } return PyLong_FromLong(old_limit); } From 6c40b592a61a8f303ded66e5fa7703c2b96cc031 Mon Sep 17 00:00:00 2001 From: AN Long Date: Thu, 16 May 2024 00:37:37 +0800 Subject: [PATCH 10/15] Update clinic generated files --- Modules/_csv.c | 2 +- Modules/clinic/_csv.c.h | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index e6e3b39f36224e..74042660b467e7 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -1651,7 +1651,7 @@ the old limit is returned static PyObject * _csv_field_size_limit_impl(PyObject *module, PyObject *new_limit) -/*[clinic end generated code: output=f2799ecd908e250b input=3e49d42e37a7d449]*/ +/*[clinic end generated code: output=f2799ecd908e250b input=cec70e9226406435]*/ { _csvstate *module_state = get_csv_state(module); long old_limit = FT_ATOMIC_LOAD_LONG(module_state->field_limit); diff --git a/Modules/clinic/_csv.c.h b/Modules/clinic/_csv.c.h index 0c9fc32e50e993..2442bdcde5679f 100644 --- a/Modules/clinic/_csv.c.h +++ b/Modules/clinic/_csv.c.h @@ -6,7 +6,6 @@ preserve # include "pycore_gc.h" // PyGC_Head # include "pycore_runtime.h" // _Py_ID() #endif -#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() #include "pycore_modsupport.h" // _PyArg_UnpackKeywords() PyDoc_STRVAR(_csv_list_dialects__doc__, @@ -202,11 +201,9 @@ _csv_field_size_limit(PyObject *module, PyObject *const *args, Py_ssize_t nargs, } new_limit = args[0]; skip_optional_pos: - Py_BEGIN_CRITICAL_SECTION(module); return_value = _csv_field_size_limit_impl(module, new_limit); - Py_END_CRITICAL_SECTION(); exit: return return_value; } -/*[clinic end generated code: output=c9376d6674176385 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=9ec59717f5414d8b input=a9049054013a1b77]*/ From a79cf88bcf60370b38c909ebf415c61ca5a41be3 Mon Sep 17 00:00:00 2001 From: AN Long Date: Fri, 17 May 2024 17:50:59 +0800 Subject: [PATCH 11/15] Apply suggestions from code review --- Include/cpython/pyatomic_msc.h | 2 +- Modules/_csv.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/cpython/pyatomic_msc.h b/Include/cpython/pyatomic_msc.h index 4d4c5a5f735fbf..e687d42c4ae69d 100644 --- a/Include/cpython/pyatomic_msc.h +++ b/Include/cpython/pyatomic_msc.h @@ -360,7 +360,7 @@ _Py_atomic_exchange_long(long *obj, long value) { _Py_atomic_ASSERT_ARG_TYPE(int32_t); return (int)_Py_atomic_exchange_int32((int32_t *)obj, - (int32_t)value); + (int32_t)value); } static inline unsigned int diff --git a/Modules/_csv.c b/Modules/_csv.c index 74042660b467e7..036acf7e60cec4 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -14,7 +14,7 @@ module instead. #endif #include "Python.h" -#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_LONG +#include "pycore_pyatomic_ft_wrappers.h" #include // offsetof() #include From 3739c46a51cf338c02eb8d34971f983e0f80a07e Mon Sep 17 00:00:00 2001 From: AN Long Date: Wed, 10 Jul 2024 21:13:43 +0800 Subject: [PATCH 12/15] Change field_limit to Py_ssize_t --- Modules/_csv.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index 6698b269dbc9d4..387ae5876c8bb6 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -35,7 +35,7 @@ typedef struct { PyTypeObject *dialect_type; PyTypeObject *reader_type; PyTypeObject *writer_type; - long field_limit; /* max parsed field size */ + Py_ssize_t field_limit; /* max parsed field size */ PyObject *str_write; } _csvstate; @@ -703,7 +703,7 @@ parse_grow_buff(ReaderObj *self) static int parse_add_char(ReaderObj *self, _csvstate *module_state, Py_UCS4 c) { - long field_limit = FT_ATOMIC_LOAD_LONG(module_state->field_limit); + Py_ssize_t field_limit = FT_ATOMIC_LOAD_SSIZE_RELAXED(module_state->field_limit); if (self->field_len >= field_limit) { PyErr_Format(module_state->error_obj, "field larger than field limit (%ld)", @@ -1654,7 +1654,7 @@ _csv_field_size_limit_impl(PyObject *module, PyObject *new_limit) /*[clinic end generated code: output=f2799ecd908e250b input=cec70e9226406435]*/ { _csvstate *module_state = get_csv_state(module); - long old_limit = FT_ATOMIC_LOAD_LONG(module_state->field_limit); + Py_ssize_t old_limit = FT_ATOMIC_LOAD_SSIZE_RELAXED(module_state->field_limit); if (new_limit != NULL) { if (!PyLong_CheckExact(new_limit)) { PyErr_Format(PyExc_TypeError, @@ -1665,7 +1665,7 @@ _csv_field_size_limit_impl(PyObject *module, PyObject *new_limit) if (new_limit_value == -1 && PyErr_Occurred()) { return NULL; } - FT_ATOMIC_STORE_LONG(module_state->field_limit, new_limit_value); + FT_ATOMIC_STORE_SSIZE_RELAXED(module_state->field_limit, new_limit_value); } return PyLong_FromLong(old_limit); } From 850bf2aed6e9ba8c0af432b1c72b3a46974a06a7 Mon Sep 17 00:00:00 2001 From: AN Long Date: Wed, 10 Jul 2024 21:16:20 +0800 Subject: [PATCH 13/15] Remove atomic functions for long type --- Include/cpython/pyatomic.h | 6 ------ Include/cpython/pyatomic_gcc.h | 8 ------- Include/cpython/pyatomic_msc.h | 21 ------------------- Include/cpython/pyatomic_std.h | 14 ------------- .../internal/pycore_pyatomic_ft_wrappers.h | 5 ----- 5 files changed, 54 deletions(-) diff --git a/Include/cpython/pyatomic.h b/Include/cpython/pyatomic.h index 5909bbc7f60899..4ecef4f56edf42 100644 --- a/Include/cpython/pyatomic.h +++ b/Include/cpython/pyatomic.h @@ -275,9 +275,6 @@ _Py_atomic_or_uintptr(uintptr_t *obj, uintptr_t value); static inline int _Py_atomic_load_int(const int *obj); -static inline long -_Py_atomic_load_long(const long *obj); - static inline int8_t _Py_atomic_load_int8(const int8_t *obj); @@ -372,9 +369,6 @@ _Py_atomic_load_ullong_relaxed(const unsigned long long *obj); static inline void _Py_atomic_store_int(int *obj, int value); -static inline void -_Py_atomic_store_long(long *obj, long value); - static inline void _Py_atomic_store_int8(int8_t *obj, int8_t value); diff --git a/Include/cpython/pyatomic_gcc.h b/Include/cpython/pyatomic_gcc.h index 89ad01f37b3149..ef09954d53ac1d 100644 --- a/Include/cpython/pyatomic_gcc.h +++ b/Include/cpython/pyatomic_gcc.h @@ -247,10 +247,6 @@ static inline int _Py_atomic_load_int(const int *obj) { return __atomic_load_n(obj, __ATOMIC_SEQ_CST); } -static inline long -_Py_atomic_load_long(const long *obj) -{ return __atomic_load_n(obj, __ATOMIC_SEQ_CST); } - static inline int8_t _Py_atomic_load_int8(const int8_t *obj) { return __atomic_load_n(obj, __ATOMIC_SEQ_CST); } @@ -373,10 +369,6 @@ static inline void _Py_atomic_store_int(int *obj, int value) { __atomic_store_n(obj, value, __ATOMIC_SEQ_CST); } -static inline void -_Py_atomic_store_long(long *obj, long value) -{ __atomic_store_n(obj, value, __ATOMIC_SEQ_CST); } - static inline void _Py_atomic_store_int8(int8_t *obj, int8_t value) { __atomic_store_n(obj, value, __ATOMIC_SEQ_CST); } diff --git a/Include/cpython/pyatomic_msc.h b/Include/cpython/pyatomic_msc.h index eab273a5ef5277..84da21bdcbff4f 100644 --- a/Include/cpython/pyatomic_msc.h +++ b/Include/cpython/pyatomic_msc.h @@ -355,14 +355,6 @@ _Py_atomic_exchange_int(int *obj, int value) (int32_t)value); } -static inline int -_Py_atomic_exchange_long(long *obj, long value) -{ - _Py_atomic_ASSERT_ARG_TYPE(int32_t); - return (int)_Py_atomic_exchange_int32((int32_t *)obj, - (int32_t)value); -} - static inline unsigned int _Py_atomic_exchange_uint(unsigned int *obj, unsigned int value) { @@ -589,13 +581,6 @@ _Py_atomic_load_int(const int *obj) return (int)_Py_atomic_load_uint32((uint32_t *)obj); } -static inline long -_Py_atomic_load_long(const long *obj) -{ - _Py_atomic_ASSERT_ARG_TYPE(uint32_t); - return (int)_Py_atomic_load_uint32((uint32_t *)obj); -} - static inline unsigned int _Py_atomic_load_uint(const unsigned int *obj) { @@ -742,12 +727,6 @@ _Py_atomic_store_int(int *obj, int value) (void)_Py_atomic_exchange_int(obj, value); } -static inline void -_Py_atomic_store_long(long *obj, long value) -{ - (void)_Py_atomic_exchange_long(obj, value); -} - static inline void _Py_atomic_store_int8(int8_t *obj, int8_t value) { diff --git a/Include/cpython/pyatomic_std.h b/Include/cpython/pyatomic_std.h index 170ce22b83e410..7c71e94c68f8e6 100644 --- a/Include/cpython/pyatomic_std.h +++ b/Include/cpython/pyatomic_std.h @@ -413,13 +413,6 @@ _Py_atomic_load_int(const int *obj) return atomic_load((const _Atomic(int)*)obj); } -static inline long -_Py_atomic_load_long(const long *obj) -{ - _Py_USING_STD; - return atomic_load((const _Atomic(long)*)obj); -} - static inline int8_t _Py_atomic_load_int8(const int8_t *obj) { @@ -644,13 +637,6 @@ _Py_atomic_store_int(int *obj, int value) atomic_store((_Atomic(int)*)obj, value); } -static inline void -_Py_atomic_store_long(long *obj, long value) -{ - _Py_USING_STD; - atomic_store((_Atomic(long)*)obj, value); -} - static inline void _Py_atomic_store_int8(int8_t *obj, int8_t value) { diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index fc50e7bab41124..a1bb383bcd22e9 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -57,9 +57,6 @@ extern "C" { _Py_atomic_store_ssize_relaxed(&value, new_value) #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \ _Py_atomic_store_uint8_relaxed(&value, new_value) -#define FT_ATOMIC_LOAD_LONG(value) _Py_atomic_load_long(&value) -#define FT_ATOMIC_STORE_LONG(value, new_value) \ - _Py_atomic_store_long(&value, new_value) #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) \ _Py_atomic_store_uint16_relaxed(&value, new_value) #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) \ @@ -86,8 +83,6 @@ extern "C" { #define FT_ATOMIC_STORE_UINTPTR_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value -#define FT_ATOMIC_LOAD_LONG(value) value -#define FT_ATOMIC_STORE_LONG(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) value = new_value From bfbfc564c2dbe25227ab0c9ebe1386e9cde0a22b Mon Sep 17 00:00:00 2001 From: AN Long Date: Wed, 10 Jul 2024 21:22:28 +0800 Subject: [PATCH 14/15] Fix types --- Modules/_csv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index 387ae5876c8bb6..c2bdb8fd8eb0f0 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -706,7 +706,7 @@ parse_add_char(ReaderObj *self, _csvstate *module_state, Py_UCS4 c) Py_ssize_t field_limit = FT_ATOMIC_LOAD_SSIZE_RELAXED(module_state->field_limit); if (self->field_len >= field_limit) { PyErr_Format(module_state->error_obj, - "field larger than field limit (%ld)", + "field larger than field limit (%zd)", field_limit); return -1; } @@ -1661,7 +1661,7 @@ _csv_field_size_limit_impl(PyObject *module, PyObject *new_limit) "limit must be an integer"); return NULL; } - long new_limit_value = PyLong_AsLong(new_limit); + Py_ssize_t new_limit_value = PyLong_AsSsize_t(new_limit); if (new_limit_value == -1 && PyErr_Occurred()) { return NULL; } From d73b8277d00546e71af980b38cca5b8e959e3a8d Mon Sep 17 00:00:00 2001 From: AN Long Date: Sun, 14 Jul 2024 23:50:16 +0800 Subject: [PATCH 15/15] Fix the return type of field_size_limit --- Modules/_csv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_csv.c b/Modules/_csv.c index c2bdb8fd8eb0f0..d716bc21ab962e 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -1667,7 +1667,7 @@ _csv_field_size_limit_impl(PyObject *module, PyObject *new_limit) } FT_ATOMIC_STORE_SSIZE_RELAXED(module_state->field_limit, new_limit_value); } - return PyLong_FromLong(old_limit); + return PyLong_FromSsize_t(old_limit); } static PyType_Slot error_slots[] = {