From 3a5cb3ce0c26f63ae59a606cffca30305b73518d Mon Sep 17 00:00:00 2001 From: alperyoney Date: Mon, 14 Jul 2025 14:50:23 -0700 Subject: [PATCH] gh-116738: Make pwd module thread-safe --- Lib/test/test_free_threading/test_pwd.py | 33 +++++++++++++++ ...-07-15-10-03-57.gh-issue-116738.oFttKl.rst | 1 + Modules/clinic/pwdmodule.c.h | 6 ++- Modules/pwdmodule.c | 40 +++++++++++++------ Tools/c-analyzer/cpython/ignored.tsv | 1 + 5 files changed, 66 insertions(+), 15 deletions(-) create mode 100644 Lib/test/test_free_threading/test_pwd.py create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-07-15-10-03-57.gh-issue-116738.oFttKl.rst diff --git a/Lib/test/test_free_threading/test_pwd.py b/Lib/test/test_free_threading/test_pwd.py new file mode 100644 index 00000000000000..58ab22bae39673 --- /dev/null +++ b/Lib/test/test_free_threading/test_pwd.py @@ -0,0 +1,33 @@ +import unittest + +from test.support import threading_helper +from test.support.threading_helper import run_concurrently + +from test import test_pwd + + +NTHREADS = 10 + + +@threading_helper.requires_working_threading() +class TestPwd(unittest.TestCase): + def setUp(self): + self.test_pwd = test_pwd.PwdTest() + + def test_racing_test_values(self): + # test_pwd.test_values() calls pwd.getpwall() and checks the entries + run_concurrently( + worker_func=self.test_pwd.test_values, nthreads=NTHREADS + ) + + def test_racing_test_values_extended(self): + # test_pwd.test_values_extended() calls pwd.getpwall(), pwd.getpwnam(), + # pwd.getpwduid() and checks the entries + run_concurrently( + worker_func=self.test_pwd.test_values_extended, + nthreads=NTHREADS, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-07-15-10-03-57.gh-issue-116738.oFttKl.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-07-15-10-03-57.gh-issue-116738.oFttKl.rst new file mode 100644 index 00000000000000..07d8b45fb7fbf1 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-07-15-10-03-57.gh-issue-116738.oFttKl.rst @@ -0,0 +1 @@ +Make functions in :mod:`pwd` thread-safe on the :term:`free threaded ` build. diff --git a/Modules/clinic/pwdmodule.c.h b/Modules/clinic/pwdmodule.c.h index 365d99aab1dd22..43d4825031c7e6 100644 --- a/Modules/clinic/pwdmodule.c.h +++ b/Modules/clinic/pwdmodule.c.h @@ -2,6 +2,8 @@ preserve [clinic start generated code]*/ +#include "pycore_modsupport.h" // _PyArg_BadArgument() + PyDoc_STRVAR(pwd_getpwuid__doc__, "getpwuid($module, uidobj, /)\n" "--\n" @@ -34,7 +36,7 @@ pwd_getpwnam(PyObject *module, PyObject *arg) PyObject *name; if (!PyUnicode_Check(arg)) { - PyErr_Format(PyExc_TypeError, "getpwnam() argument must be str, not %T", arg); + _PyArg_BadArgument("getpwnam", "argument", "str", arg); goto exit; } name = arg; @@ -71,4 +73,4 @@ pwd_getpwall(PyObject *module, PyObject *Py_UNUSED(ignored)) #ifndef PWD_GETPWALL_METHODDEF #define PWD_GETPWALL_METHODDEF #endif /* !defined(PWD_GETPWALL_METHODDEF) */ -/*[clinic end generated code: output=dac88d500f6d6f49 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=5a8fb12939ff4ea3 input=a9049054013a1b77]*/ diff --git a/Modules/pwdmodule.c b/Modules/pwdmodule.c index c5a8cead19a773..ac1a04a54edd60 100644 --- a/Modules/pwdmodule.c +++ b/Modules/pwdmodule.c @@ -1,12 +1,6 @@ /* UNIX password file access module */ -// Need limited C API version 3.13 for PyMem_RawRealloc() -#include "pyconfig.h" // Py_GIL_DISABLED -#ifndef Py_GIL_DISABLED -# define Py_LIMITED_API 0x030d0000 -#endif - #include "Python.h" #include "posixmodule.h" @@ -69,6 +63,11 @@ get_pwd_state(PyObject *module) static struct PyModuleDef pwdmodule; +/* Mutex to protect calls to getpwuid(), getpwnam(), and getpwent(). + * These functions return pointer to static data structure, which + * may be overwritten by any subsequent calls. */ +static PyMutex pwd_db_mutex = {0}; + #define DEFAULT_BUFFER_SIZE 1024 static PyObject * @@ -182,9 +181,15 @@ pwd_getpwuid(PyObject *module, PyObject *uidobj) Py_END_ALLOW_THREADS #else + PyMutex_Lock(&pwd_db_mutex); + // The getpwuid() function is not required to be thread-safe. + // https://pubs.opengroup.org/onlinepubs/009604499/functions/getpwuid.html p = getpwuid(uid); #endif if (p == NULL) { +#ifndef HAVE_GETPWUID_R + PyMutex_Unlock(&pwd_db_mutex); +#endif PyMem_RawFree(buf); if (nomem == 1) { return PyErr_NoMemory(); @@ -200,6 +205,8 @@ pwd_getpwuid(PyObject *module, PyObject *uidobj) retval = mkpwent(module, p); #ifdef HAVE_GETPWUID_R PyMem_RawFree(buf); +#else + PyMutex_Unlock(&pwd_db_mutex); #endif return retval; } @@ -265,9 +272,15 @@ pwd_getpwnam_impl(PyObject *module, PyObject *name) Py_END_ALLOW_THREADS #else + PyMutex_Lock(&pwd_db_mutex); + // The getpwnam() function is not required to be thread-safe. + // https://pubs.opengroup.org/onlinepubs/009604599/functions/getpwnam.html p = getpwnam(name_chars); #endif if (p == NULL) { +#ifndef HAVE_GETPWNAM_R + PyMutex_Unlock(&pwd_db_mutex); +#endif if (nomem == 1) { PyErr_NoMemory(); } @@ -278,6 +291,9 @@ pwd_getpwnam_impl(PyObject *module, PyObject *name) goto out; } retval = mkpwent(module, p); +#ifndef HAVE_GETPWNAM_R + PyMutex_Unlock(&pwd_db_mutex); +#endif out: PyMem_RawFree(buf); Py_DECREF(bytes); @@ -302,12 +318,12 @@ pwd_getpwall_impl(PyObject *module) if ((d = PyList_New(0)) == NULL) return NULL; -#ifdef Py_GIL_DISABLED - static PyMutex getpwall_mutex = {0}; - PyMutex_Lock(&getpwall_mutex); -#endif + PyMutex_Lock(&pwd_db_mutex); int failure = 0; PyObject *v = NULL; + // The setpwent(), getpwent() and endpwent() functions are not required to + // be thread-safe. + // https://pubs.opengroup.org/onlinepubs/009696799/functions/setpwent.html setpwent(); while ((p = getpwent()) != NULL) { v = mkpwent(module, p); @@ -321,9 +337,7 @@ pwd_getpwall_impl(PyObject *module) done: endpwent(); -#ifdef Py_GIL_DISABLED - PyMutex_Unlock(&getpwall_mutex); -#endif + PyMutex_Unlock(&pwd_db_mutex); if (failure) { Py_XDECREF(v); Py_CLEAR(d); diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index 64a9f11a944176..dc626e4bea0f59 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -168,6 +168,7 @@ Python/sysmodule.c - _preinit_xoptions - Modules/faulthandler.c faulthandler_dump_traceback reentrant - Modules/faulthandler.c faulthandler_dump_c_stack reentrant - Modules/grpmodule.c - group_db_mutex - +Modules/pwdmodule.c - pwd_db_mutex - Python/pylifecycle.c _Py_FatalErrorFormat reentrant - Python/pylifecycle.c fatal_error reentrant -