Skip to content

Commit b23d657

Browse files
authored
[3.14] gh-158893: Make os.strerror() thread-safe (#159120) (#159145)
[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) (cherry picked from commit c774d15)
1 parent f37ebb2 commit b23d657

12 files changed

Lines changed: 272 additions & 18 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: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,8 +440,9 @@ def parse_config_h(fp, vars=None):
440440
if vars is None:
441441
vars = {}
442442
import re
443-
define_rx = re.compile("#define ([A-Z][A-Za-z0-9_]+) (.*)\n")
444-
undef_rx = re.compile("/[*] #undef ([A-Z][A-Za-z0-9_]+) [*]/\n")
443+
name_rx = '(?:[A-Z]|_Py_)[A-Za-z0-9_]+'
444+
define_rx = re.compile(fr"#define ({name_rx}) (.*)\n")
445+
undef_rx = re.compile(fr"/[*] #undef ({name_rx}) [*]/\n")
445446

446447
while True:
447448
line = fp.readline()
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
@@ -28,7 +28,7 @@
2828
get_path, get_path_names, _INSTALL_SCHEMES,
2929
get_default_scheme, get_scheme_names, get_config_var,
3030
_expand_vars, _get_preferred_schemes,
31-
is_python_build, _PROJECT_BASE)
31+
is_python_build, _PROJECT_BASE, parse_config_h)
3232
from sysconfig.__main__ import _main, _parse_makefile, _get_pybuilddir, _get_json_data_name
3333
import _imp
3434
import _osx_support
@@ -749,6 +749,72 @@ def test_sysconfig_config_vars_no_prefix_cache(self):
749749
self.assertEqual(config_vars['exec_prefix'], sys.exec_prefix)
750750
self.assertEqual(config_vars['platbase'], sys.exec_prefix)
751751

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

753819
class MakefileTests(unittest.TestCase):
754820

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/posixmodule.c‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13276,13 +13276,7 @@ static PyObject *
1327613276
os_strerror_impl(PyObject *module, int code)
1327713277
/*[clinic end generated code: output=baebf09fa02a78f2 input=75a8673d97915a91]*/
1327813278
{
13279-
char *message = strerror(code);
13280-
if (message == NULL) {
13281-
PyErr_SetString(PyExc_ValueError,
13282-
"strerror() argument out of range");
13283-
return NULL;
13284-
}
13285-
return PyUnicode_DecodeLocale(message, "surrogateescape");
13279+
return _Py_strerror(code);
1328613280
}
1328713281

1328813282

‎Python/errors.c‎

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

842842
#ifndef MS_WINDOWS
843843
if (i != 0) {
844-
const char *s = strerror(i);
845-
message = PyUnicode_DecodeLocale(s, "surrogateescape");
844+
message = _Py_strerror(i);
846845
}
847846
else {
848847
/* Sometimes errno didn't get set */

‎Python/fileutils.c‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3136,3 +3136,97 @@ _Py_IsValidFD(int fd)
31363136
return (fstat(fd, &st) == 0);
31373137
#endif
31383138
}
3139+
3140+
3141+
// Call strerror_r(code) if available, or use strerror() otherwise. Decode the
3142+
// result from the locale encoding using surrogateescape error handler.
3143+
//
3144+
// On success, return a Unicode string. On error, set an exception and return
3145+
// NULL.
3146+
PyObject*
3147+
_Py_strerror(int code)
3148+
/*[clinic end generated code: output=baebf09fa02a78f2 input=75a8673d97915a91]*/
3149+
{
3150+
const char *errors = "surrogateescape";
3151+
3152+
#ifdef _Py_HAVE_STRERROR_R
3153+
// Check which strerror_r() API is used
3154+
# if defined(__GLIBC__) && !((_POSIX_C_SOURCE >= 200112L) && !defined(_GNU_SOURCE))
3155+
# define Py_STRERROR_R_GNU
3156+
# elif defined(__ANDROID__) && defined(_GNU_SOURCE)
3157+
# define Py_STRERROR_R_GNU
3158+
# endif
3159+
#endif
3160+
3161+
#ifdef Py_STRERROR_R_GNU
3162+
// Implementation for the GNU flavor of strerror_r()
3163+
3164+
// On Linux, the longest translated strerror() message is 86 bytes
3165+
// (including the NUL byte).
3166+
char buffer[100];
3167+
char *message = strerror_r(code, buffer, Py_ARRAY_LENGTH(buffer));
3168+
// The strerror_r() GNU flavor doesn't provide a way to check if the error
3169+
// message was truncated or not.
3170+
//
3171+
// When the buffer is used, a trailing NUL byte is always written.
3172+
assert(message != buffer || memchr(buffer, 0, Py_ARRAY_LENGTH(buffer)) != NULL);
3173+
return PyUnicode_DecodeLocale(message, errors);
3174+
3175+
#elif defined(_Py_HAVE_STRERROR_R)
3176+
// Implementation for the XSI-compliant flavor of strerror_r()
3177+
3178+
// On Linux and FreeBSD, the longest translated strerror() message is 86
3179+
// bytes (including the NUL byte).
3180+
char small_buffer[100];
3181+
size_t buflen = Py_ARRAY_LENGTH(small_buffer);
3182+
char *buffer = NULL;
3183+
#ifndef NDEBUG
3184+
// Make sure that strerror_r() writes a trailing null byte
3185+
small_buffer[buflen - 1] = '#';
3186+
#endif
3187+
int len = strerror_r(code, small_buffer, buflen);
3188+
if (len == ERANGE) {
3189+
while (len == ERANGE) {
3190+
if (buflen > (size_t)PY_SSIZE_T_MAX / 2) {
3191+
PyMem_Free(buffer);
3192+
PyErr_NoMemory();
3193+
return NULL;
3194+
}
3195+
buflen = buflen * 2;
3196+
3197+
char *new_buffer = PyMem_Realloc(buffer, buflen);
3198+
if (new_buffer == NULL) {
3199+
PyMem_Free(buffer);
3200+
PyErr_NoMemory();
3201+
return NULL;
3202+
}
3203+
buffer = new_buffer;
3204+
#ifndef NDEBUG
3205+
buffer[buflen - 1] = '#';
3206+
#endif
3207+
len = strerror_r(code, buffer, buflen);
3208+
}
3209+
}
3210+
else {
3211+
buffer = small_buffer;
3212+
}
3213+
3214+
// strerror_r() always writes a trailing NUL byte
3215+
assert(memchr(buffer, 0, buflen) != NULL);
3216+
PyObject *result = PyUnicode_DecodeLocale(buffer, errors);
3217+
if (buffer != small_buffer) {
3218+
PyMem_Free(buffer);
3219+
}
3220+
return result;
3221+
3222+
#else
3223+
// strerror() implementation (usually not thread-safe)
3224+
char *message = strerror(code);
3225+
if (message == NULL) {
3226+
PyErr_SetString(PyExc_ValueError,
3227+
"strerror() argument out of range");
3228+
return NULL;
3229+
}
3230+
return PyUnicode_DecodeLocale(message, errors);
3231+
#endif
3232+
}

‎Python/remote_debugging.c‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
#include "pyconfig.h"
33

44
#include "Python.h"
5-
#include "internal/pycore_runtime.h"
6-
#include "internal/pycore_ceval.h"
5+
#include "pycore_runtime.h"
6+
#include "pycore_ceval.h"
7+
#include "pycore_fileutils.h" // _Py_strerror()
78

89
#if defined(Py_REMOTE_DEBUG) && defined(Py_SUPPORTS_REMOTE_DEBUG)
910
#include "remote_debug.h"
@@ -121,10 +122,14 @@ write_memory(proc_handle_t *handle, uintptr_t remote_address, size_t len, const
121122
}
122123
errno = err;
123124
PyErr_SetFromErrno(PyExc_OSError);
124-
_set_debug_exception_cause(PyExc_OSError,
125-
"process_vm_writev failed for PID %d at address 0x%lx "
126-
"(size %zu, partial write %zd bytes): %s",
127-
handle->pid, remote_address + result, len - result, result, strerror(err));
125+
PyObject *message = _Py_strerror(err);
126+
if (message != NULL) {
127+
_set_debug_exception_cause(PyExc_OSError,
128+
"process_vm_writev failed for PID %d at address 0x%lx "
129+
"(size %zu, partial write %zd bytes): %s",
130+
handle->pid, remote_address + result, len - result, result, message);
131+
Py_DECREF(message);
132+
}
128133
return -1;
129134
}
130135

‎configure‎

Lines changed: 44 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)