gh-126316: Make grp.getgrall() thread-safe: add a mutex

[vstinner]

grpmodule.c is no longer built with the limited C API, since PyMutex is excluded from the limited C API.

• Issue: grp is not thread safe #126316

[vstinner]

@ZeroIntensity@colesbury: Would you mind to review my change?

[ZeroIntensity]

Hmm, this is an interesting way to do it. Local static variables are notorious for being non-thread-safe, so I'm not sure this kind of approach will work. If it does, I don't think we're fixing any issues with re-entrancy.

I would just go with @critical_section on each function, and then switch the module's Py_mod_multiple_interpreters to Py_MOD_MULTIPLE_INTERPRETERS_SUPPORTED instead of Py_MOD_PER_INTERPRETER_GIL_SUPPORTED to denote shared-GIL for subinterpreters. That should fix races for both cases.

[colesbury]

This approach looks good to me. I think you are right that we don't have to worry about deadlock here because the calls inside the lock won't call arbitrary Python code.

There's a failing CI related to generated files. I think the pycore_lock.h include in pycore_modsupport.h can be removed now. Also, maybe the arg clinic files need to be regenerated?

Local static variables are notorious for being non-thread-safe

A static PyMutex is fine because PyMutex is thread-safe. (It doesn't matter if it's a local or global static variable.)

The general problems with static variables (including static local variables) is that the data structures are not thread-safe or that they are lazily initialized pointers, which is not the case here.

[ZeroIntensity]

Thanks for the insight, Sam. How scalable is this approach (as in, will we have a static PyMutex everywhere)?

[colesbury]

I don't think we'll have static PyMutex everywhere, but this is a good pattern for certain libc functions that are not thread-safe and do not have thread-safe alternatives.

[vstinner]

I think the pycore_lock.h include in pycore_modsupport.h can be removed now.

Correct: I wrote #127093 to remove it.

[ZeroIntensity]

LGTM

[vstinner]

Merged, thanks for reviews @colesbury and @ZeroIntensity.

[bedevere-app]

GH-127104 is a backport of this pull request to the 3.13 branch.