Skip to content

Commit c774d15

Browse files
authored
[3.15] gh-158893: Make os.strerror() thread-safe (#159120)
* gh-157695: Parse also _Py_HAVE_xxx variables in sysconfig (#158926) Add parse_config_h() tests to test_sysconfig. (cherry picked from commit 978a8be) * gh-158893: Make os.strerror() thread-safe (#158927) Make os.strerror() thread-safe: use the reentrant strerror_r() function if available. * The configure script now checks if strerror_r() is supported. * Add a stress test to test_free_threading.test_os (new module). * Add a comment on decode_current_locale() assertion which fails if the input string is mutated. * Add an assertion to _Py_DecodeLocale() to detect if the input string was mutated during the function call. * gh-158893: Add internal _Py_strerror() function (#158982) Add a new internal _Py_strerror() function to Python/fileutils.c. It uses strerror_r() if available, or use strerror() otherwise. Replace all strerror(code) calls with _Py_strerror(code). (cherry picked from commit 15dd735)
1 parent 2ffa96d commit c774d15

13 files changed

Lines changed: 315 additions & 32 deletions

File tree

‎Include/internal/pycore_fileutils.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,9 @@ extern int _Py_GetTicksPerSecond(long *ticks_per_second);
314314
// Export for '_testcapi' shared extension
315315
PyAPI_FUNC(int) _Py_IsValidFD(int fd);
316316

317+
// Export for '_remote_debugging' shared extension
318+
PyAPI_FUNC(PyObject*) _Py_strerror(int code);
319+
317320
#ifdef __cplusplus
318321
}
319322
#endif

‎Lib/sysconfig/__init__.py‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,9 +435,10 @@ def parse_config_h(fp, vars=None):
435435
if vars is None:
436436
vars = {}
437437
import re
438-
define_rx = re.compile("#define ([A-Z][A-Za-z0-9_]+) (.*)\n")
439-
undef_rx = re.compile("/[*] #undef ([A-Z][A-Za-z0-9_]+) [*]/\n")
440-
quoted_re = re.compile('^"(.*)"$')
438+
name_rx = '(?:[A-Z]|_Py_)[A-Za-z0-9_]+'
439+
define_rx = re.compile(fr"#define ({name_rx}) (.*)\n")
440+
undef_rx = re.compile(fr"/[*] #undef ({name_rx}) [*]/\n")
441+
quoted_re = re.compile(r'"(.*)"')
441442

442443
while True:
443444
line = fp.readline()
@@ -446,7 +447,7 @@ def parse_config_h(fp, vars=None):
446447
m = define_rx.match(line)
447448
if m:
448449
n, v = m.group(1, 2)
449-
if mq := quoted_re.match(v):
450+
if mq := quoted_re.fullmatch(v):
450451
v = mq.group(1)
451452
try:
452453
if n in _ALWAYS_STR:
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import errno
2+
import os
3+
import sysconfig
4+
import unittest
5+
6+
from test.support import threading_helper
7+
from test.support.threading_helper import run_concurrently
8+
9+
10+
NTHREADS = 10
11+
12+
13+
@threading_helper.requires_working_threading()
14+
class TestOs(unittest.TestCase):
15+
@unittest.skipUnless(sysconfig.get_config_var('_Py_HAVE_STRERROR_R'),
16+
'need _Py_HAVE_STRERROR_R macro')
17+
def test_strerror(self):
18+
# gh-158893: os.strerror() is implemented with strerror_r() which is
19+
# thread safe. Well, check if it's actually the case.
20+
last_error = max([getattr(errno, name) for name in dir(errno)
21+
if name.startswith('E')])
22+
test_errors = tuple(range(1, last_error + 1))
23+
loops = 20
24+
25+
def worker():
26+
for _ in range(loops):
27+
for i in test_errors:
28+
os.strerror(i)
29+
30+
run_concurrently(
31+
worker_func=worker, nthreads=NTHREADS
32+
)
33+
34+
35+
if __name__ == "__main__":
36+
unittest.main()

‎Lib/test/test_sysconfig.py‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
get_path, get_path_names, _INSTALL_SCHEMES,
3030
get_default_scheme, get_scheme_names, get_config_var,
3131
_expand_vars, _get_preferred_schemes,
32-
is_python_build, _PROJECT_BASE)
32+
is_python_build, _PROJECT_BASE, parse_config_h)
3333
from sysconfig.__main__ import _main, _parse_makefile, _get_pybuilddir, _get_json_data_name
3434
import _imp
3535
import _osx_support
@@ -760,6 +760,72 @@ def test_sysconfig_config_vars_no_prefix_cache(self):
760760
self.assertEqual(config_vars['exec_prefix'], sys.exec_prefix)
761761
self.assertEqual(config_vars['platbase'], sys.exec_prefix)
762762

763+
def test_parse_config_h(self):
764+
config = textwrap.dedent('''
765+
#ifndef Py_PYCONFIG_H
766+
#define Py_PYCONFIG_H
767+
768+
/* C comment */
769+
770+
#define ALIGNOF_LONG 8
771+
#define HAVE_ACCEPT 1
772+
#define _Py_HAVE_COSPI 1
773+
#define INVALID_NUMBER abc
774+
#define ALT_SOABI "cpython-316t-x86_64-linux-gnu"
775+
776+
// Undef macros must be written as "/* #undef NAME */":
777+
// name must be valid and there is not value.
778+
/* #undef ANDROID_API_LEVEL */
779+
#undef IGNORE_UNDEF
780+
/* #undef IGNORE_VALUE 1 */
781+
782+
# _ALWAYS_STR: don't convert values to an integer,
783+
# but quotes are removed
784+
#define IPHONEOS_DEPLOYMENT_TARGET "13.0"
785+
#define MACOSX_DEPLOYMENT_TARGET 10
786+
787+
// Spaces are tolerated after the name, not before
788+
#define SPACES_AFTER 1
789+
#define IGNORED_SPACES_BEFORE 1
790+
791+
// Ignore macro without value
792+
#define IGNORE_NO_VALUE
793+
794+
// Ignore macros with an invalid name
795+
#define _PRIVATE_IGNORED 1
796+
#define aLOWER_IGNORED 1
797+
#define 123IGNORED 1
798+
#define INVALID-NAME 1
799+
#define INVALID#NAME 1
800+
#define NONASCII_NAME_é 1
801+
802+
// Ignore single letter names
803+
#define A 1
804+
/* #undef A */
805+
806+
#endif /*Py_PYCONFIG_H*/
807+
''')
808+
809+
filename = TESTFN
810+
self.addCleanup(unlink, filename)
811+
with open(filename, "w", encoding="utf-8") as fp:
812+
fp.write(config)
813+
vars = {}
814+
with open(filename, encoding="utf-8") as fp:
815+
parse_config_h(fp, vars)
816+
expected = {
817+
'ALIGNOF_LONG': 8,
818+
'HAVE_ACCEPT': 1,
819+
'_Py_HAVE_COSPI': 1,
820+
'INVALID_NUMBER': 'abc',
821+
'ALT_SOABI': 'cpython-316t-x86_64-linux-gnu',
822+
'ANDROID_API_LEVEL': 0,
823+
'IPHONEOS_DEPLOYMENT_TARGET': '13.0',
824+
'MACOSX_DEPLOYMENT_TARGET': '10', # str, not int
825+
'SPACES_AFTER': 1,
826+
}
827+
self.assertEqual(vars, expected)
828+
763829

764830
class MakefileTests(unittest.TestCase):
765831

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Make :func:`os.strerror` thread-safe: use the reentrant ``strerror_r()``
2+
function if available. Patch by Victor Stinner.

‎Modules/_remote_debugging/subprocess.c‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
******************************************************************************/
77

88
#include "_remote_debugging.h"
9+
#include "pycore_fileutils.h" // _Py_strerror()
910

1011
#ifndef MS_WINDOWS
1112
#include <unistd.h>
@@ -229,9 +230,13 @@ get_child_pids_platform(pid_t target_pid, int recursive, pid_array_t *result)
229230
if (entry == NULL) {
230231
if (errno != 0) {
231232
int err = errno;
232-
_set_debug_oserror_from_errno_with_filename(err, "/proc",
233-
"Failed to read process directory '/proc': %s",
234-
strerror(err));
233+
PyObject *message = _Py_strerror(err);
234+
if (message != NULL) {
235+
_set_debug_oserror_from_errno_with_filename(err, "/proc",
236+
"Failed to read process directory '/proc': %S",
237+
message);
238+
Py_DECREF(message);
239+
}
235240
goto done;
236241
}
237242
break;
@@ -259,9 +264,13 @@ get_child_pids_platform(pid_t target_pid, int recursive, pid_array_t *result)
259264
if (closedir(proc_dir) != 0) {
260265
int err = errno;
261266
proc_dir = NULL;
262-
_set_debug_oserror_from_errno_with_filename(err, "/proc",
263-
"Failed to close process directory '/proc': %s",
264-
strerror(err));
267+
PyObject *message = _Py_strerror(err);
268+
if (message != NULL) {
269+
_set_debug_oserror_from_errno_with_filename(err, "/proc",
270+
"Failed to close process directory '/proc': %S",
271+
message);
272+
Py_DECREF(message);
273+
}
265274
goto done;
266275
}
267276
proc_dir = NULL;

‎Modules/_remote_debugging/threads.c‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
******************************************************************************/
77

88
#include "_remote_debugging.h"
9+
#include "pycore_fileutils.h" // _Py_strerror()
910

1011
#ifndef MS_WINDOWS
1112
#include <unistd.h>
@@ -680,9 +681,13 @@ read_thread_ids(RemoteUnwinderObject *unwinder, _Py_RemoteDebug_ThreadsState *st
680681
int err = errno;
681682
closedir(dir);
682683
_Py_RemoteDebug_InitThreadsState(unwinder, st);
683-
_set_debug_oserror_from_errno_with_filename(err, task_path,
684-
"Failed to read process task directory '%s': %s",
685-
task_path, strerror(err));
684+
PyObject *message = _Py_strerror(err);
685+
if (message != NULL) {
686+
_set_debug_oserror_from_errno_with_filename(err, task_path,
687+
"Failed to read process task directory '%s': %S",
688+
task_path, message);
689+
Py_DECREF(message);
690+
}
686691
return -1;
687692
}
688693
break;
@@ -713,9 +718,13 @@ read_thread_ids(RemoteUnwinderObject *unwinder, _Py_RemoteDebug_ThreadsState *st
713718
if (closedir(dir) != 0) {
714719
int err = errno;
715720
_Py_RemoteDebug_InitThreadsState(unwinder, st);
716-
_set_debug_oserror_from_errno_with_filename(err, task_path,
717-
"Failed to close process task directory '%s': %s",
718-
task_path, strerror(err));
721+
PyObject *message = _Py_strerror(err);
722+
if (message != NULL) {
723+
_set_debug_oserror_from_errno_with_filename(err, task_path,
724+
"Failed to close process task directory '%s': %S",
725+
task_path, message);
726+
Py_DECREF(message);
727+
}
719728
return -1;
720729
}
721730
st->tids = unwinder->thread_tids;
@@ -780,8 +789,12 @@ _Py_RemoteDebug_StopAllThreads(RemoteUnwinderObject *unwinder, _Py_RemoteDebug_T
780789
}
781790
if (ret < 0) {
782791
detach_threads(st, seized);
783-
_set_debug_oserror_from_errno(err,
784-
"Failed to seize thread %d: %s", tid, strerror(err));
792+
PyObject *message = _Py_strerror(err);
793+
if (message != NULL) {
794+
_set_debug_oserror_from_errno(err,
795+
"Failed to seize thread %d: %S", tid, message);
796+
Py_DECREF(message);
797+
}
785798
_Py_RemoteDebug_InitThreadsState(unwinder, st);
786799
return -1;
787800
}
@@ -791,8 +804,12 @@ _Py_RemoteDebug_StopAllThreads(RemoteUnwinderObject *unwinder, _Py_RemoteDebug_T
791804
err = errno;
792805
if (err != ESRCH) {
793806
detach_threads(st, seized);
794-
_set_debug_oserror_from_errno(err,
795-
"Failed to interrupt thread %d: %s", tid, strerror(err));
807+
PyObject *message = _Py_strerror(err);
808+
if (message != NULL) {
809+
_set_debug_oserror_from_errno(err,
810+
"Failed to interrupt thread %d: %S", tid, message);
811+
Py_DECREF(message);
812+
}
796813
_Py_RemoteDebug_InitThreadsState(unwinder, st);
797814
return -1;
798815
}
@@ -803,8 +820,12 @@ _Py_RemoteDebug_StopAllThreads(RemoteUnwinderObject *unwinder, _Py_RemoteDebug_T
803820
err = errno;
804821
if (err != ECHILD && err != ESRCH) {
805822
detach_threads(st, seized);
806-
_set_debug_oserror_from_errno(err,
807-
"waitpid failed for thread %d: %s", tid, strerror(err));
823+
PyObject *message = _Py_strerror(err);
824+
if (message != NULL) {
825+
_set_debug_oserror_from_errno(err,
826+
"waitpid failed for thread %d: %S", tid, message);
827+
Py_DECREF(message);
828+
}
808829
_Py_RemoteDebug_InitThreadsState(unwinder, st);
809830
return -1;
810831
}

‎Modules/posixmodule.c‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13831,13 +13831,7 @@ static PyObject *
1383113831
os_strerror_impl(PyObject *module, int code)
1383213832
/*[clinic end generated code: output=baebf09fa02a78f2 input=75a8673d97915a91]*/
1383313833
{
13834-
char *message = strerror(code);
13835-
if (message == NULL) {
13836-
PyErr_SetString(PyExc_ValueError,
13837-
"strerror() argument out of range");
13838-
return NULL;
13839-
}
13840-
return PyUnicode_DecodeLocale(message, "surrogateescape");
13834+
return _Py_strerror(code);
1384113835
}
1384213836

1384313837

‎Python/errors.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -829,8 +829,7 @@ PyErr_SetFromErrnoWithFilenameObjects(PyObject *exc, PyObject *filenameObject, P
829829

830830
#ifndef MS_WINDOWS
831831
if (i != 0) {
832-
const char *s = strerror(i);
833-
message = PyUnicode_DecodeLocale(s, "surrogateescape");
832+
message = _Py_strerror(i);
834833
}
835834
else {
836835
/* Sometimes errno didn't get set */

0 commit comments

Comments
 (0)