gh-125243: Fix ZoneInfo data race in free threading build (#125281)

Lock `ZoneInfoType` to protect accesses to `ZONEINFO_STRONG_CACHE`.
Refactor the `tp_new` handler to use Argument Clinic so that we can just
use `@critical_section` annotations on the relevant functions.

Also use `PyDict_SetDefaultRef` instead of `PyDict_SetDefault` when
inserting into the `TIMEDELTA_CACHE`.
This commit is contained in:
Sam Gross 2024-10-13 16:17:51 -04:00 committed by GitHub
parent cb8e5995d8
commit f1d33dbddd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 88 additions and 21 deletions

View File

@ -0,0 +1,2 @@
Fix data race when creating :class:`zoneinfo.ZoneInfo` objects in the free
threading build.

View File

@ -3,6 +3,7 @@
#endif #endif
#include "Python.h" #include "Python.h"
#include "pycore_critical_section.h" // _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED()
#include "pycore_long.h" // _PyLong_GetOne() #include "pycore_long.h" // _PyLong_GetOne()
#include "pycore_pyerrors.h" // _PyErr_ChainExceptions1() #include "pycore_pyerrors.h" // _PyErr_ChainExceptions1()
@ -298,15 +299,20 @@ get_weak_cache(zoneinfo_state *state, PyTypeObject *type)
} }
} }
static PyObject * /*[clinic input]
zoneinfo_new(PyTypeObject *type, PyObject *args, PyObject *kw) @critical_section
{ @classmethod
PyObject *key = NULL; zoneinfo.ZoneInfo.__new__
static char *kwlist[] = {"key", NULL};
if (PyArg_ParseTupleAndKeywords(args, kw, "O", kwlist, &key) == 0) {
return NULL;
}
key: object
Create a new ZoneInfo instance.
[clinic start generated code]*/
static PyObject *
zoneinfo_ZoneInfo_impl(PyTypeObject *type, PyObject *key)
/*[clinic end generated code: output=95e61dab86bb95c3 input=ef73d7a83bf8790e]*/
{
zoneinfo_state *state = zoneinfo_get_state_by_self(type); zoneinfo_state *state = zoneinfo_get_state_by_self(type);
PyObject *instance = zone_from_strong_cache(state, type, key); PyObject *instance = zone_from_strong_cache(state, type, key);
if (instance != NULL || PyErr_Occurred()) { if (instance != NULL || PyErr_Occurred()) {
@ -467,6 +473,7 @@ zoneinfo_ZoneInfo_no_cache_impl(PyTypeObject *type, PyTypeObject *cls,
} }
/*[clinic input] /*[clinic input]
@critical_section
@classmethod @classmethod
zoneinfo.ZoneInfo.clear_cache zoneinfo.ZoneInfo.clear_cache
@ -481,7 +488,7 @@ Clear the ZoneInfo cache.
static PyObject * static PyObject *
zoneinfo_ZoneInfo_clear_cache_impl(PyTypeObject *type, PyTypeObject *cls, zoneinfo_ZoneInfo_clear_cache_impl(PyTypeObject *type, PyTypeObject *cls,
PyObject *only_keys) PyObject *only_keys)
/*[clinic end generated code: output=114d9b7c8a22e660 input=e32ca3bb396788ba]*/ /*[clinic end generated code: output=114d9b7c8a22e660 input=35944715df26d24e]*/
{ {
zoneinfo_state *state = zoneinfo_get_state_by_cls(cls); zoneinfo_state *state = zoneinfo_get_state_by_cls(cls);
PyObject *weak_cache = get_weak_cache(state, type); PyObject *weak_cache = get_weak_cache(state, type);
@ -816,14 +823,10 @@ zoneinfo_ZoneInfo__unpickle_impl(PyTypeObject *type, PyTypeObject *cls,
/*[clinic end generated code: output=556712fc709deecb input=6ac8c73eed3de316]*/ /*[clinic end generated code: output=556712fc709deecb input=6ac8c73eed3de316]*/
{ {
if (from_cache) { if (from_cache) {
PyObject *val_args = PyTuple_Pack(1, key); PyObject *rv;
if (val_args == NULL) { Py_BEGIN_CRITICAL_SECTION(type);
return NULL; rv = zoneinfo_ZoneInfo_impl(type, key);
} Py_END_CRITICAL_SECTION();
PyObject *rv = zoneinfo_new(type, val_args, NULL);
Py_DECREF(val_args);
return rv; return rv;
} }
else { else {
@ -858,8 +861,7 @@ load_timedelta(zoneinfo_state *state, long seconds)
0, seconds, 0, 1, PyDateTimeAPI->DeltaType); 0, seconds, 0, 1, PyDateTimeAPI->DeltaType);
if (tmp != NULL) { if (tmp != NULL) {
rv = PyDict_SetDefault(state->TIMEDELTA_CACHE, pyoffset, tmp); PyDict_SetDefaultRef(state->TIMEDELTA_CACHE, pyoffset, tmp, &rv);
Py_XINCREF(rv);
Py_DECREF(tmp); Py_DECREF(tmp);
} }
} }
@ -2368,6 +2370,7 @@ strong_cache_free(StrongCacheNode *root)
static void static void
remove_from_strong_cache(zoneinfo_state *state, StrongCacheNode *node) remove_from_strong_cache(zoneinfo_state *state, StrongCacheNode *node)
{ {
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(state->ZoneInfoType);
if (state->ZONEINFO_STRONG_CACHE == node) { if (state->ZONEINFO_STRONG_CACHE == node) {
state->ZONEINFO_STRONG_CACHE = node->next; state->ZONEINFO_STRONG_CACHE = node->next;
} }
@ -2422,6 +2425,7 @@ eject_from_strong_cache(zoneinfo_state *state, const PyTypeObject *const type,
return 0; return 0;
} }
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(state->ZoneInfoType);
StrongCacheNode *cache = state->ZONEINFO_STRONG_CACHE; StrongCacheNode *cache = state->ZONEINFO_STRONG_CACHE;
StrongCacheNode *node = find_in_strong_cache(cache, key); StrongCacheNode *node = find_in_strong_cache(cache, key);
if (node != NULL) { if (node != NULL) {
@ -2478,6 +2482,7 @@ zone_from_strong_cache(zoneinfo_state *state, const PyTypeObject *const type,
return NULL; // Strong cache currently only implemented for base class return NULL; // Strong cache currently only implemented for base class
} }
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(state->ZoneInfoType);
StrongCacheNode *cache = state->ZONEINFO_STRONG_CACHE; StrongCacheNode *cache = state->ZONEINFO_STRONG_CACHE;
StrongCacheNode *node = find_in_strong_cache(cache, key); StrongCacheNode *node = find_in_strong_cache(cache, key);
@ -2504,6 +2509,7 @@ update_strong_cache(zoneinfo_state *state, const PyTypeObject *const type,
return; return;
} }
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(state->ZoneInfoType);
StrongCacheNode *new_node = strong_cache_node_new(key, zone); StrongCacheNode *new_node = strong_cache_node_new(key, zone);
if (new_node == NULL) { if (new_node == NULL) {
return; return;
@ -2631,7 +2637,7 @@ static PyType_Slot zoneinfo_slots[] = {
{Py_tp_getattro, PyObject_GenericGetAttr}, {Py_tp_getattro, PyObject_GenericGetAttr},
{Py_tp_methods, zoneinfo_methods}, {Py_tp_methods, zoneinfo_methods},
{Py_tp_members, zoneinfo_members}, {Py_tp_members, zoneinfo_members},
{Py_tp_new, zoneinfo_new}, {Py_tp_new, zoneinfo_ZoneInfo},
{Py_tp_dealloc, zoneinfo_dealloc}, {Py_tp_dealloc, zoneinfo_dealloc},
{Py_tp_traverse, zoneinfo_traverse}, {Py_tp_traverse, zoneinfo_traverse},
{Py_tp_clear, zoneinfo_clear}, {Py_tp_clear, zoneinfo_clear},

View File

@ -6,8 +6,65 @@ preserve
# include "pycore_gc.h" // PyGC_Head # include "pycore_gc.h" // PyGC_Head
# include "pycore_runtime.h" // _Py_ID() # include "pycore_runtime.h" // _Py_ID()
#endif #endif
#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION()
#include "pycore_modsupport.h" // _PyArg_UnpackKeywords() #include "pycore_modsupport.h" // _PyArg_UnpackKeywords()
PyDoc_STRVAR(zoneinfo_ZoneInfo__doc__,
"ZoneInfo(key)\n"
"--\n"
"\n"
"Create a new ZoneInfo instance.");
static PyObject *
zoneinfo_ZoneInfo_impl(PyTypeObject *type, PyObject *key);
static PyObject *
zoneinfo_ZoneInfo(PyTypeObject *type, PyObject *args, PyObject *kwargs)
{
PyObject *return_value = NULL;
#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)
#define NUM_KEYWORDS 1
static struct {
PyGC_Head _this_is_not_used;
PyObject_VAR_HEAD
PyObject *ob_item[NUM_KEYWORDS];
} _kwtuple = {
.ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS)
.ob_item = { &_Py_ID(key), },
};
#undef NUM_KEYWORDS
#define KWTUPLE (&_kwtuple.ob_base.ob_base)
#else // !Py_BUILD_CORE
# define KWTUPLE NULL
#endif // !Py_BUILD_CORE
static const char * const _keywords[] = {"key", NULL};
static _PyArg_Parser _parser = {
.keywords = _keywords,
.fname = "ZoneInfo",
.kwtuple = KWTUPLE,
};
#undef KWTUPLE
PyObject *argsbuf[1];
PyObject * const *fastargs;
Py_ssize_t nargs = PyTuple_GET_SIZE(args);
PyObject *key;
fastargs = _PyArg_UnpackKeywords(_PyTuple_CAST(args)->ob_item, nargs, kwargs, NULL, &_parser, 1, 1, 0, argsbuf);
if (!fastargs) {
goto exit;
}
key = fastargs[0];
Py_BEGIN_CRITICAL_SECTION(type);
return_value = zoneinfo_ZoneInfo_impl(type, key);
Py_END_CRITICAL_SECTION();
exit:
return return_value;
}
PyDoc_STRVAR(zoneinfo_ZoneInfo_from_file__doc__, PyDoc_STRVAR(zoneinfo_ZoneInfo_from_file__doc__,
"from_file($type, file_obj, /, key=None)\n" "from_file($type, file_obj, /, key=None)\n"
"--\n" "--\n"
@ -182,7 +239,9 @@ zoneinfo_ZoneInfo_clear_cache(PyTypeObject *type, PyTypeObject *cls, PyObject *c
} }
only_keys = args[0]; only_keys = args[0];
skip_optional_kwonly: skip_optional_kwonly:
Py_BEGIN_CRITICAL_SECTION(type);
return_value = zoneinfo_ZoneInfo_clear_cache_impl(type, cls, only_keys); return_value = zoneinfo_ZoneInfo_clear_cache_impl(type, cls, only_keys);
Py_END_CRITICAL_SECTION();
exit: exit:
return return_value; return return_value;
@ -372,4 +431,4 @@ zoneinfo_ZoneInfo__unpickle(PyTypeObject *type, PyTypeObject *cls, PyObject *con
exit: exit:
return return_value; return return_value;
} }
/*[clinic end generated code: output=2a15f32fdd2ab6cd input=a9049054013a1b77]*/ /*[clinic end generated code: output=b4fdc0b30247110a input=a9049054013a1b77]*/