https://github.com/python/cpython/commit/3c2bd66e21bd8de69a89ebf09ff9d8e78ddfb839
commit: 3c2bd66e21bd8de69a89ebf09ff9d8e78ddfb839
branch: main
author: Victor Stinner <[email protected]>
committer: vstinner <[email protected]>
date: 2024-11-21T15:47:24+01:00
summary:
gh-126316: Make grp.getgrall() thread-safe: add a mutex (#127055)
grpmodule.c is no longer built with the limited C API, since PyMutex
is excluded from the limited C API.
files:
A Misc/NEWS.d/next/Library/2024-11-20-11-37-08.gh-issue-126316.ElkZmE.rst
M Include/internal/pycore_global_objects_fini_generated.h
M Include/internal/pycore_global_strings.h
M Include/internal/pycore_runtime_init_generated.h
M Include/internal/pycore_unicodeobject_generated.h
M Modules/clinic/grpmodule.c.h
M Modules/grpmodule.c
M Tools/c-analyzer/cpython/ignored.tsv
diff --git a/Include/internal/pycore_global_objects_fini_generated.h
b/Include/internal/pycore_global_objects_fini_generated.h
index e4f0138e17edfa..c12e242d560bde 100644
--- a/Include/internal/pycore_global_objects_fini_generated.h
+++ b/Include/internal/pycore_global_objects_fini_generated.h
@@ -982,6 +982,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) {
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(hi));
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(hook));
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(hour));
+ _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(id));
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(ident));
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(identity_hint));
_PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(ignore));
diff --git a/Include/internal/pycore_global_strings.h
b/Include/internal/pycore_global_strings.h
index e70f11e2a26cd5..dfd9f2b799ec8e 100644
--- a/Include/internal/pycore_global_strings.h
+++ b/Include/internal/pycore_global_strings.h
@@ -471,6 +471,7 @@ struct _Py_global_strings {
STRUCT_FOR_ID(hi)
STRUCT_FOR_ID(hook)
STRUCT_FOR_ID(hour)
+ STRUCT_FOR_ID(id)
STRUCT_FOR_ID(ident)
STRUCT_FOR_ID(identity_hint)
STRUCT_FOR_ID(ignore)
diff --git a/Include/internal/pycore_runtime_init_generated.h
b/Include/internal/pycore_runtime_init_generated.h
index 5d404c8fd91ca6..b631382cae058a 100644
--- a/Include/internal/pycore_runtime_init_generated.h
+++ b/Include/internal/pycore_runtime_init_generated.h
@@ -980,6 +980,7 @@ extern "C" {
INIT_ID(hi), \
INIT_ID(hook), \
INIT_ID(hour), \
+ INIT_ID(id), \
INIT_ID(ident), \
INIT_ID(identity_hint), \
INIT_ID(ignore), \
diff --git a/Include/internal/pycore_unicodeobject_generated.h
b/Include/internal/pycore_unicodeobject_generated.h
index d0bc8d7186c053..24cec3a4fded7a 100644
--- a/Include/internal/pycore_unicodeobject_generated.h
+++ b/Include/internal/pycore_unicodeobject_generated.h
@@ -1680,6 +1680,10 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp)
{
_PyUnicode_InternStatic(interp, &string);
assert(_PyUnicode_CheckConsistency(string, 1));
assert(PyUnicode_GET_LENGTH(string) != 1);
+ string = &_Py_ID(id);
+ _PyUnicode_InternStatic(interp, &string);
+ assert(_PyUnicode_CheckConsistency(string, 1));
+ assert(PyUnicode_GET_LENGTH(string) != 1);
string = &_Py_ID(ident);
_PyUnicode_InternStatic(interp, &string);
assert(_PyUnicode_CheckConsistency(string, 1));
diff --git
a/Misc/NEWS.d/next/Library/2024-11-20-11-37-08.gh-issue-126316.ElkZmE.rst
b/Misc/NEWS.d/next/Library/2024-11-20-11-37-08.gh-issue-126316.ElkZmE.rst
new file mode 100644
index 00000000000000..d643254c5b3564
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2024-11-20-11-37-08.gh-issue-126316.ElkZmE.rst
@@ -0,0 +1,2 @@
+:mod:`grp`: Make :func:`grp.getgrall` thread-safe by adding a mutex. Patch
+by Victor Stinner.
diff --git a/Modules/clinic/grpmodule.c.h b/Modules/clinic/grpmodule.c.h
index cc0ad210f42743..facfa3a43e490e 100644
--- a/Modules/clinic/grpmodule.c.h
+++ b/Modules/clinic/grpmodule.c.h
@@ -2,6 +2,12 @@
preserve
[clinic start generated code]*/
+#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)
+# include "pycore_gc.h" // PyGC_Head
+# include "pycore_runtime.h" // _Py_ID()
+#endif
+#include "pycore_modsupport.h" // _PyArg_UnpackKeywords()
+
PyDoc_STRVAR(grp_getgrgid__doc__,
"getgrgid($module, /, id)\n"
"--\n"
@@ -11,21 +17,49 @@ PyDoc_STRVAR(grp_getgrgid__doc__,
"If id is not valid, raise KeyError.");
#define GRP_GETGRGID_METHODDEF \
- {"getgrgid", (PyCFunction)(void(*)(void))grp_getgrgid,
METH_VARARGS|METH_KEYWORDS, grp_getgrgid__doc__},
+ {"getgrgid", _PyCFunction_CAST(grp_getgrgid), METH_FASTCALL|METH_KEYWORDS,
grp_getgrgid__doc__},
static PyObject *
grp_getgrgid_impl(PyObject *module, PyObject *id);
static PyObject *
-grp_getgrgid(PyObject *module, PyObject *args, PyObject *kwargs)
+grp_getgrgid(PyObject *module, PyObject *const *args, Py_ssize_t nargs,
PyObject *kwnames)
{
PyObject *return_value = NULL;
- static char *_keywords[] = {"id", NULL};
+ #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)
+
+ #define NUM_KEYWORDS 1
+ static struct {
+ PyGC_Head _this_is_not_used;
+ PyObject_VAR_HEAD
+ PyObject *ob_item[NUM_KEYWORDS];
+ } _kwtuple = {
+ .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS)
+ .ob_item = { &_Py_ID(id), },
+ };
+ #undef NUM_KEYWORDS
+ #define KWTUPLE (&_kwtuple.ob_base.ob_base)
+
+ #else // !Py_BUILD_CORE
+ # define KWTUPLE NULL
+ #endif // !Py_BUILD_CORE
+
+ static const char * const _keywords[] = {"id", NULL};
+ static _PyArg_Parser _parser = {
+ .keywords = _keywords,
+ .fname = "getgrgid",
+ .kwtuple = KWTUPLE,
+ };
+ #undef KWTUPLE
+ PyObject *argsbuf[1];
PyObject *id;
- if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O:getgrgid", _keywords,
- &id))
+ args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser,
+ /*minpos*/ 1, /*maxpos*/ 1, /*minkw*/ 0, /*varpos*/ 0, argsbuf);
+ if (!args) {
goto exit;
+ }
+ id = args[0];
return_value = grp_getgrgid_impl(module, id);
exit:
@@ -41,21 +75,53 @@ PyDoc_STRVAR(grp_getgrnam__doc__,
"If name is not valid, raise KeyError.");
#define GRP_GETGRNAM_METHODDEF \
- {"getgrnam", (PyCFunction)(void(*)(void))grp_getgrnam,
METH_VARARGS|METH_KEYWORDS, grp_getgrnam__doc__},
+ {"getgrnam", _PyCFunction_CAST(grp_getgrnam), METH_FASTCALL|METH_KEYWORDS,
grp_getgrnam__doc__},
static PyObject *
grp_getgrnam_impl(PyObject *module, PyObject *name);
static PyObject *
-grp_getgrnam(PyObject *module, PyObject *args, PyObject *kwargs)
+grp_getgrnam(PyObject *module, PyObject *const *args, Py_ssize_t nargs,
PyObject *kwnames)
{
PyObject *return_value = NULL;
- static char *_keywords[] = {"name", NULL};
+ #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)
+
+ #define NUM_KEYWORDS 1
+ static struct {
+ PyGC_Head _this_is_not_used;
+ PyObject_VAR_HEAD
+ PyObject *ob_item[NUM_KEYWORDS];
+ } _kwtuple = {
+ .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS)
+ .ob_item = { &_Py_ID(name), },
+ };
+ #undef NUM_KEYWORDS
+ #define KWTUPLE (&_kwtuple.ob_base.ob_base)
+
+ #else // !Py_BUILD_CORE
+ # define KWTUPLE NULL
+ #endif // !Py_BUILD_CORE
+
+ static const char * const _keywords[] = {"name", NULL};
+ static _PyArg_Parser _parser = {
+ .keywords = _keywords,
+ .fname = "getgrnam",
+ .kwtuple = KWTUPLE,
+ };
+ #undef KWTUPLE
+ PyObject *argsbuf[1];
PyObject *name;
- if (!PyArg_ParseTupleAndKeywords(args, kwargs, "U:getgrnam", _keywords,
- &name))
+ args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser,
+ /*minpos*/ 1, /*maxpos*/ 1, /*minkw*/ 0, /*varpos*/ 0, argsbuf);
+ if (!args) {
+ goto exit;
+ }
+ if (!PyUnicode_Check(args[0])) {
+ _PyArg_BadArgument("getgrnam", "argument 'name'", "str", args[0]);
goto exit;
+ }
+ name = args[0];
return_value = grp_getgrnam_impl(module, name);
exit:
@@ -82,4 +148,4 @@ grp_getgrall(PyObject *module, PyObject *Py_UNUSED(ignored))
{
return grp_getgrall_impl(module);
}
-/*[clinic end generated code: output=81f180beb67fc585 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=2154194308dab038 input=a9049054013a1b77]*/
diff --git a/Modules/grpmodule.c b/Modules/grpmodule.c
index f7d3e12f347ec2..29da9936b65504 100644
--- a/Modules/grpmodule.c
+++ b/Modules/grpmodule.c
@@ -1,9 +1,8 @@
/* UNIX group 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
+// Argument Clinic uses the internal C API
+#ifndef Py_BUILD_CORE_BUILTIN
+# define Py_BUILD_CORE_MODULE 1
#endif
#include "Python.h"
@@ -281,23 +280,33 @@ static PyObject *
grp_getgrall_impl(PyObject *module)
/*[clinic end generated code: output=585dad35e2e763d7 input=d7df76c825c367df]*/
{
- PyObject *d;
- struct group *p;
-
- if ((d = PyList_New(0)) == NULL)
+ PyObject *d = PyList_New(0);
+ if (d == NULL) {
return NULL;
+ }
+
+ static PyMutex getgrall_mutex = {0};
+ PyMutex_Lock(&getgrall_mutex);
setgrent();
+
+ struct group *p;
while ((p = getgrent()) != NULL) {
+ // gh-126316: Don't release the mutex around mkgrent() since
+ // setgrent()/endgrent() are not reentrant / thread-safe. A deadlock
+ // is unlikely since mkgrent() should not be able to call arbitrary
+ // Python code.
PyObject *v = mkgrent(module, p);
if (v == NULL || PyList_Append(d, v) != 0) {
Py_XDECREF(v);
- Py_DECREF(d);
- endgrent();
- return NULL;
+ Py_CLEAR(d);
+ goto done;
}
Py_DECREF(v);
}
+
+done:
endgrent();
+ PyMutex_Unlock(&getgrall_mutex);
return d;
}
diff --git a/Tools/c-analyzer/cpython/ignored.tsv
b/Tools/c-analyzer/cpython/ignored.tsv
index 686f3935d91bda..4327a111eedbaf 100644
--- a/Tools/c-analyzer/cpython/ignored.tsv
+++ b/Tools/c-analyzer/cpython/ignored.tsv
@@ -739,6 +739,7 @@ Modules/expat/xmlrole.c - declClose -
Modules/expat/xmlrole.c - error -
## other
+Modules/grpmodule.c grp_getgrall_impl getgrall_mutex -
Modules/_io/_iomodule.c - _PyIO_Module -
Modules/_sqlite/module.c - _sqlite3module -
Modules/clinic/md5module.c.h _md5_md5 _keywords -
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]
