gh-116738: Make _csv module thread-safe
This extension module has some module state values which may be read or written in different threads:
field_limit: A simple integer value to store the maximum field size for the parser;dialects: A dict value to store all the dialects by string names.
field_limit
There is a public function field_size_limit that can read and write it, using a critical section to protect it. For other internal usages, _Py_atomic_load_long and _Py_atomic_store_long are used to make it thread safe.
dialects
There are three public functions get_dialect, unregister_dialect and list_dialects that can access this field. For the first two functions, a critical section is used to protect it.
And the list_dialects function just returns the dict's keys with PyDict_Keys. I think its result is readonly and PyDict_Keys itself is thread safe, so we don't need to do anything.
Other internal usages of dialects involve accessing its value with get_dialect_from_registry, which simply calls PyDict_GetItemRef and returns the result. I think we don't need to change it too.
Update: I think these APIs is just call dict APIs which is already thread-safe, and there are no other state changes inside these APIs, and no intermediate variables need to protect in these functions, so we don't need add locks on them.
- Issue: gh-116738
Sorry for the long delay in reviewing this. Two requests:
* Let's change `field_limit` to a `Py_ssize_t`. That seems more appropriate here and avoids the need for adding new atomic bindings. I'd prefer to avoid adding additional atomic bindings because its more code to maintain and its easy to add subtle bugs in the atomic bindings. Additionally, `field_limit` is compared to `field_len`, which is already a `Py_ssize_t`. * Let's use relaxed atomics for reading and writing `field_limit`, e.g., `FT_ATOMIC_STORE_SSIZE_RELAXED`. Relaxed ordering is fine for these sorts of global settings.For the data type change:
* `%ld` -> `%zd` * `PyLong_AsLong` -> `PyLong_AsSsize_t`
Thanks for the review, updated!
Thanks @aisk for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. 🐍🍒⛏🤖
GH-125328 is a backport of this pull request to the 3.13 branch.