Skip to content

gh-130421: Fix data race on timebase initialization #130592

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 3 commits into from
Feb 27, 2025
Merged

gh-130421: Fix data race on timebase initialization #130592

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f614171
gh-130421: Fix data race on timebase initialization
colesbury Feb 26, 2025
e8199e4
timebase -> base
colesbury Feb 26, 2025
b876b43
PyStatus_Error -> _PyStatus_ERR
colesbury Feb 27, 2025
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
2 changes: 2 additions & 0 deletions Include/internal/pycore_runtime.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions Include/internal/pycore_time.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions Python/pystate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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();
Expand Down
72 changes: 24 additions & 48 deletions Python/pytime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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.base);
info->adjustable = 1;
}

Expand Down Expand Up @@ -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.
Expand All @@ -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");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use _PyStatus_ERR() instead: it stores the name of the current function, which gives more information in case of failure.

Suggested change
return PyStatus_Error("invalid QueryPerformanceFrequency");
return _PyStatus_ERR("invalid QueryPerformanceFrequency");

Same remark below (py_mach_timebase_info).

}
return 0;
return PyStatus_Ok();
}


Expand All @@ -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;
}
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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;
}
Expand All @@ -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)
Expand Down
Loading