Skip to content

gh-116738: Make _csv module thread-safe #118344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Oct 11, 2024
Merged

gh-116738: Make _csv module thread-safe #118344

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
eeba1c2
Make the filed_limit thread safe
aisk Apr 27, 2024
40e71a8
Make dialects fields on module state thread safe
aisk Apr 27, 2024
c77750f
Utilize FT_ATOMIC_LOAD_INT32
aisk Apr 27, 2024
6fbcb9f
Remove critical section on `dialects` field
aisk Apr 27, 2024
60753f1
Totally revert the change to csv_register_dialect
aisk Apr 29, 2024
64b7d20
Revert field_limit to long, and add atomic api for it
aisk Apr 29, 2024
1de333e
Add _Py_atomic_load_long for other platforms
aisk Apr 29, 2024
7bbfd36
Merge branch 'main' into csv-thread-safe
aisk May 15, 2024
0a41927
Revert the type change to old_limit
aisk May 15, 2024
f413092
Using atomic_store instead of critical_section
aisk May 15, 2024
6c40b59
Update clinic generated files
aisk May 15, 2024
a79cf88
Apply suggestions from code review
aisk May 17, 2024
1f03de1
Merge branch 'main' into csv-thread-safe
aisk Jul 10, 2024
3739c46
Change field_limit to Py_ssize_t
aisk Jul 10, 2024
850bf2a
Remove atomic functions for long type
aisk Jul 10, 2024
bfbfc56
Fix types
aisk Jul 10, 2024
d73b827
Fix the return type of field_size_limit
aisk Jul 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Include/cpython/pyatomic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -369,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);

Expand Down
8 changes: 8 additions & 0 deletions Include/cpython/pyatomic_gcc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
Expand Down Expand Up @@ -369,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); }
Expand Down
21 changes: 21 additions & 0 deletions Include/cpython/pyatomic_msc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -581,6 +589,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)
{
Expand Down Expand Up @@ -727,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)
{
Expand Down
14 changes: 14 additions & 0 deletions Include/cpython/pyatomic_std.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -637,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)
{
Expand Down
5 changes: 5 additions & 0 deletions Include/internal/pycore_pyatomic_ft_wrappers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ 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) \
Expand All @@ -80,6 +83,8 @@ 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

Expand Down
14 changes: 8 additions & 6 deletions Modules/_csv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module instead.
#endif

#include "Python.h"
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_LONG

#include <stddef.h> // offsetof()
#include <stdbool.h>
Expand Down Expand Up @@ -702,10 +703,11 @@ 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) {
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)",
module_state->field_limit);
field_limit);
return -1;
}
if (self->field_len == self->field_size && !parse_grow_buff(self))
Expand Down Expand Up @@ -1652,18 +1654,18 @@ _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 = 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);
}
Expand Down
Loading