From f614171f8b73274df1c58ea9f46f73953124c9fa Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 26 Feb 2025 11:25:32 -0500 Subject: [PATCH 1/3] gh-130421: Fix data race on timebase initialization Windows and macOS require precomputing a "timebase" in order to convert OS timestamps into nanoseconds. Retrieve and compute this value during runtime initialization to avoid data races when accessing the time. --- Include/internal/pycore_runtime.h | 2 + Include/internal/pycore_time.h | 12 ++++++ Python/pystate.c | 6 +++ Python/pytime.c | 72 +++++++++++-------------------- 4 files changed, 44 insertions(+), 48 deletions(-) diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index cf123791eba9ac..9324ce9eb89d2c 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -23,6 +23,7 @@ extern "C" { #include "pycore_pymem.h" // struct _pymem_allocators #include "pycore_pythread.h" // struct _pythread_runtime_state #include "pycore_signal.h" // struct _signals_runtime_state +#include "pycore_time.h" // struct _PyTime_runtime_state #include "pycore_tracemalloc.h" // struct _tracemalloc_runtime_state #include "pycore_typeobject.h" // struct _types_runtime_state #include "pycore_unicodeobject.h" // struct _Py_unicode_runtime_state @@ -168,6 +169,7 @@ typedef struct pyruntimestate { struct _Py_float_runtime_state float_state; struct _Py_unicode_runtime_state unicode_state; struct _types_runtime_state types; + struct _Py_time_runtime_state time; #if defined(__EMSCRIPTEN__) && defined(PY_CALL_TRAMPOLINE) // Used in "Python/emscripten_trampoline.c" to choose between type diff --git a/Include/internal/pycore_time.h b/Include/internal/pycore_time.h index 205ac5d3781ddd..833987fd639344 100644 --- a/Include/internal/pycore_time.h +++ b/Include/internal/pycore_time.h @@ -331,6 +331,18 @@ extern double _PyTimeFraction_Resolution( const _PyTimeFraction *frac); +// --- _Py_time_runtime_state ------------------------------------------------ + +struct _Py_time_runtime_state { +#if defined(MS_WINDOWS) || defined(__APPLE__) + _PyTimeFraction base; +#else + char _unused; +#endif +}; + +extern PyStatus _PyTime_Init(struct _Py_time_runtime_state *state); + #ifdef __cplusplus } #endif diff --git a/Python/pystate.c b/Python/pystate.c index 09b83cdeb1f42d..9ebd9fdea60746 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -20,6 +20,7 @@ #include "pycore_pystate.h" #include "pycore_runtime_init.h" // _PyRuntimeState_INIT #include "pycore_stackref.h" // Py_STACKREF_DEBUG +#include "pycore_time.h" // _PyTime_Init() #include "pycore_obmalloc.h" // _PyMem_obmalloc_state_on_heap() #include "pycore_uniqueid.h" // _PyObject_FinalizePerThreadRefcounts() @@ -461,6 +462,11 @@ _PyRuntimeState_Init(_PyRuntimeState *runtime) assert(!runtime->_initialized); } + PyStatus status = _PyTime_Init(&runtime->time); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + if (gilstate_tss_init(runtime) != 0) { _PyRuntimeState_Fini(runtime); return _PyStatus_NO_MEMORY(); diff --git a/Python/pytime.c b/Python/pytime.c index c039fc98ce4bde..4a8e3ec6dcdea5 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -56,14 +56,6 @@ #endif -#ifdef MS_WINDOWS -static _PyTimeFraction py_qpc_base = {0, 0}; - -// Forward declaration -static int py_win_perf_counter_frequency(_PyTimeFraction *base, int raise_exc); -#endif - - static PyTime_t _PyTime_GCD(PyTime_t x, PyTime_t y) { @@ -923,15 +915,9 @@ py_get_system_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) if (info) { // GetSystemTimePreciseAsFileTime() is implemented using // QueryPerformanceCounter() internally. - if (py_qpc_base.denom == 0) { - if (py_win_perf_counter_frequency(&py_qpc_base, raise_exc) < 0) { - return -1; - } - } - info->implementation = "GetSystemTimePreciseAsFileTime()"; info->monotonic = 0; - info->resolution = _PyTimeFraction_Resolution(&py_qpc_base); + info->resolution = _PyTimeFraction_Resolution(&_PyRuntime.time.timebase); info->adjustable = 1; } @@ -1043,8 +1029,8 @@ _PyTime_TimeWithInfo(PyTime_t *t, _Py_clock_info_t *info) #ifdef MS_WINDOWS -static int -py_win_perf_counter_frequency(_PyTimeFraction *base, int raise_exc) +static PyStatus +py_win_perf_counter_frequency(_PyTimeFraction *base) { LARGE_INTEGER freq; // Since Windows XP, the function cannot fail. @@ -1062,13 +1048,9 @@ py_win_perf_counter_frequency(_PyTimeFraction *base, int raise_exc) // * 10,000,000 (10 MHz): 100 ns resolution // * 3,579,545 Hz (3.6 MHz): 279 ns resolution if (_PyTimeFraction_Set(base, SEC_TO_NS, denom) < 0) { - if (raise_exc) { - PyErr_SetString(PyExc_RuntimeError, - "invalid QueryPerformanceFrequency"); - } - return -1; + return PyStatus_Error("invalid QueryPerformanceFrequency"); } - return 0; + return PyStatus_Ok(); } @@ -1078,15 +1060,9 @@ py_get_win_perf_counter(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) { assert(info == NULL || raise_exc); - if (py_qpc_base.denom == 0) { - if (py_win_perf_counter_frequency(&py_qpc_base, raise_exc) < 0) { - return -1; - } - } - if (info) { info->implementation = "QueryPerformanceCounter()"; - info->resolution = _PyTimeFraction_Resolution(&py_qpc_base); + info->resolution = _PyTimeFraction_Resolution(&_PyRuntime.time.base); info->monotonic = 1; info->adjustable = 0; } @@ -1102,15 +1078,15 @@ py_get_win_perf_counter(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) "LONGLONG is larger than PyTime_t"); ticks = (PyTime_t)ticksll; - *tp = _PyTimeFraction_Mul(ticks, &py_qpc_base); + *tp = _PyTimeFraction_Mul(ticks, &_PyRuntime.time.base); return 0; } #endif // MS_WINDOWS #ifdef __APPLE__ -static int -py_mach_timebase_info(_PyTimeFraction *base, int raise_exc) +static PyStatus +py_mach_timebase_info(_PyTimeFraction *base) { mach_timebase_info_data_t timebase; // According to the Technical Q&A QA1398, mach_timebase_info() cannot @@ -1132,16 +1108,23 @@ py_mach_timebase_info(_PyTimeFraction *base, int raise_exc) // * (1000000000, 33333335) on PowerPC: ~30 ns // * (1000000000, 25000000) on PowerPC: 40 ns if (_PyTimeFraction_Set(base, numer, denom) < 0) { - if (raise_exc) { - PyErr_SetString(PyExc_RuntimeError, - "invalid mach_timebase_info"); - } - return -1; + return PyStatus_Error("invalid mach_timebase_info"); } - return 0; + return PyStatus_Ok(); } #endif +PyStatus +_PyTime_Init(struct _Py_time_runtime_state *state) +{ +#ifdef MS_WINDOWS + return py_win_perf_counter_frequency(&state->base); +#elif defined(__APPLE__) + return py_mach_timebase_info(&state->base); +#else + return PyStatus_Ok(); +#endif +} // N.B. If raise_exc=0, this may be called without a thread state. static int @@ -1158,16 +1141,9 @@ py_get_monotonic_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) return -1; } #elif defined(__APPLE__) - static _PyTimeFraction base = {0, 0}; - if (base.denom == 0) { - if (py_mach_timebase_info(&base, raise_exc) < 0) { - return -1; - } - } - if (info) { info->implementation = "mach_absolute_time()"; - info->resolution = _PyTimeFraction_Resolution(&base); + info->resolution = _PyTimeFraction_Resolution(&_PyRuntime.time.base); info->monotonic = 1; info->adjustable = 0; } @@ -1177,7 +1153,7 @@ py_get_monotonic_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) assert(uticks <= (uint64_t)PyTime_MAX); PyTime_t ticks = (PyTime_t)uticks; - PyTime_t ns = _PyTimeFraction_Mul(ticks, &base); + PyTime_t ns = _PyTimeFraction_Mul(ticks, &_PyRuntime.time.base); *tp = ns; #elif defined(__hpux) From e8199e4ffe6932f54df35fb7f8232d88d5120032 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 26 Feb 2025 13:58:19 -0500 Subject: [PATCH 2/3] timebase -> base --- Python/pytime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/pytime.c b/Python/pytime.c index 4a8e3ec6dcdea5..3896ab26b8d991 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -917,7 +917,7 @@ py_get_system_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) // QueryPerformanceCounter() internally. info->implementation = "GetSystemTimePreciseAsFileTime()"; info->monotonic = 0; - info->resolution = _PyTimeFraction_Resolution(&_PyRuntime.time.timebase); + info->resolution = _PyTimeFraction_Resolution(&_PyRuntime.time.base); info->adjustable = 1; } From b876b43fb5eecb33affa2b08d14ffe54bc31e9e8 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 27 Feb 2025 08:02:58 -0500 Subject: [PATCH 3/3] PyStatus_Error -> _PyStatus_ERR --- Python/pytime.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Python/pytime.c b/Python/pytime.c index 3896ab26b8d991..b10d5cf927b7e1 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -1,4 +1,5 @@ #include "Python.h" +#include "pycore_initconfig.h" // _PyStatus_ERR #include "pycore_time.h" // PyTime_t #include "pycore_pystate.h" // _Py_AssertHoldsTstate() @@ -1048,7 +1049,7 @@ py_win_perf_counter_frequency(_PyTimeFraction *base) // * 10,000,000 (10 MHz): 100 ns resolution // * 3,579,545 Hz (3.6 MHz): 279 ns resolution if (_PyTimeFraction_Set(base, SEC_TO_NS, denom) < 0) { - return PyStatus_Error("invalid QueryPerformanceFrequency"); + return _PyStatus_ERR("invalid QueryPerformanceFrequency"); } return PyStatus_Ok(); } @@ -1108,7 +1109,7 @@ py_mach_timebase_info(_PyTimeFraction *base) // * (1000000000, 33333335) on PowerPC: ~30 ns // * (1000000000, 25000000) on PowerPC: 40 ns if (_PyTimeFraction_Set(base, numer, denom) < 0) { - return PyStatus_Error("invalid mach_timebase_info"); + return _PyStatus_ERR("invalid mach_timebase_info"); } return PyStatus_Ok(); }