Skip to content

long_from_non_binary_base isn't thread-safe with free threading #130599

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.

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

Closed
colesbury opened this issue Feb 26, 2025 · 3 comments
Closed

long_from_non_binary_base isn't thread-safe with free threading #130599

colesbury opened this issue Feb 26, 2025 · 3 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-free-threading topic-subinterpreters type-bug An unexpected behavior, bug, or error

Comments

@colesbury
Copy link
Contributor

colesbury commented Feb 26, 2025

Bug report

The long_from_non_binary_base function in longobject.c isn't thread-safe with free threading or per-interpreter GIL due to the initialization of log_base_BASE, convwidth_base, and convmultmax_base:

cpython/Objects/longobject.c

Lines 2835 to 2856 in f963239

static double log_base_BASE[37] = {0.0e0,};
static int convwidth_base[37] = {0,};
static twodigits convmultmax_base[37] = {0,};
if (log_base_BASE[base] == 0.0) {
twodigits convmax = base;
int i = 1;
log_base_BASE[base] = (log((double)base) /
log((double)PyLong_BASE));
for (;;) {
twodigits next = convmax * base;
if (next > PyLong_BASE) {
break;
}
convmax = next;
++i;
}
convmultmax_base[base] = convmax;
assert(i > 0);
convwidth_base[base] = i;
}

There are a few ways we could make this thread-safe:

  • Make the initialization thread-safe with something like _PyOnceFlag
  • Pre-compute the values and hardcode them.

Originally reported by @ngoldbaum in #130421

cc @tim-one

Linked PRs

@colesbury colesbury added topic-free-threading topic-subinterpreters type-bug An unexpected behavior, bug, or error labels Feb 26, 2025
colesbury added a commit to colesbury/cpython that referenced this issue Feb 26, 2025
The global initialization wasn't thread-safe with free threading or
per-interpreter GIL. Use a `_PyOnceFlag` to initialize the entries in a
thread-safe manner.
colesbury added a commit to colesbury/cpython that referenced this issue Feb 26, 2025
The global initialization wasn't thread-safe with free threading or
per-interpreter GIL. Use a `_PyOnceFlag` to initialize the entries in a
thread-safe manner.
@kumaraditya303
Copy link
Contributor

How about computing and initializing that table at runtime initialization? That can be done in _PyLong_InitTypes.

@colesbury
Copy link
Contributor Author

I haven't benchmarked the time it takes to compute the table, but I don't want to slow down Python startup, especially when most of the table entries will not be used in most programs.

If we don't want to lazily initialize the values, I'd prefer to just precompute and hardcode them.

@picnixz picnixz added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Feb 28, 2025
@nascheme
Copy link
Member

nascheme commented Mar 1, 2025

They are very fast to compute, takes less than 1 microsecond on my machine. If we hard code the table, it perhaps needs entries for different values of PyLong_BASE (usually 15 or 30 but I think it was changed in the past). So, I think the simple thing to do is compute them all at runtime init. I'l make a patch that does that.

nascheme added a commit to nascheme/cpython that referenced this issue Mar 1, 2025
This avoids a data race in free-threaded builds due to mutating the
statically arrays at runtime.  Instead, compute and initialize the constants
at runtime initialization.
nascheme added a commit to nascheme/cpython that referenced this issue Mar 3, 2025
nascheme added a commit that referenced this issue Mar 4, 2025
Avoid a data race in free-threaded builds due to mutating global arrays at
runtime.  Instead, compute the constants with an external Python script and
then define them as static global constant arrays.  These constants are
used by `long_from_non_binary_base()`.
@nascheme nascheme closed this as completed Mar 4, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in Subinterpreters Mar 4, 2025
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
…-130714)

Avoid a data race in free-threaded builds due to mutating global arrays at
runtime.  Instead, compute the constants with an external Python script and
then define them as static global constant arrays.  These constants are
used by `long_from_non_binary_base()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-free-threading topic-subinterpreters type-bug An unexpected behavior, bug, or error
Projects
Status: Done
Development

No branches or pull requests

4 participants