Skip to content

Commit eb23db2

Browse files
committed
gh-158893: Make os.strerror() thread-safe
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.
1 parent ea0ee92 commit eb23db2

7 files changed

Lines changed: 166 additions & 1 deletion

File tree

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()
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: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13923,13 +13923,78 @@ static PyObject *
1392313923
os_strerror_impl(PyObject *module, int code)
1392413924
/*[clinic end generated code: output=baebf09fa02a78f2 input=75a8673d97915a91]*/
1392513925
{
13926+
#if (defined(_Py_HAVE_STRERROR_R) && defined(__GLIBC__) \
13927+
&& !((_POSIX_C_SOURCE >= 200112L) && !defined(_GNU_SOURCE)))
13928+
// Implementation for the glibc GNU flavor of strerror_r()
13929+
13930+
// On Linux, the longest translated strerror() message is 86 bytes
13931+
// (including the NUL byte).
13932+
char buffer[100];
13933+
char *message = strerror_r(code, buffer, Py_ARRAY_LENGTH(buffer));
13934+
// The strerror_r() GNU flavor doesn't provide a way to check if the error
13935+
// message was truncated or not.
13936+
//
13937+
// When the buffer is used, a trailing NUL byte is always written.
13938+
assert(message != buffer || memchr(buffer, 0, Py_ARRAY_LENGTH(buffer)) != NULL);
13939+
return PyUnicode_DecodeLocale(message, "surrogateescape");
13940+
13941+
#elif defined(_Py_HAVE_STRERROR_R)
13942+
// Implementation for the XSI-compliant flavor of strerror_r()
13943+
13944+
// On Linux and FreeBSD, the longest translated strerror() message is 86
13945+
// bytes (including the NUL byte).
13946+
char small_buffer[100];
13947+
size_t buflen = Py_ARRAY_LENGTH(small_buffer);
13948+
char *buffer = NULL;
13949+
#ifndef NDEBUG
13950+
// Make sure that strerror_r() writes a trailing null byte
13951+
small_buffer[buflen - 1] = '#';
13952+
#endif
13953+
int len = strerror_r(code, small_buffer, buflen);
13954+
if (len == ERANGE) {
13955+
while (len == ERANGE) {
13956+
if (buflen > (size_t)PY_SSIZE_T_MAX / 2) {
13957+
PyMem_Free(buffer);
13958+
PyErr_NoMemory();
13959+
return NULL;
13960+
}
13961+
buflen = buflen * 2;
13962+
13963+
char *new_buffer = PyMem_Realloc(buffer, buflen);
13964+
if (new_buffer == NULL) {
13965+
PyMem_Free(buffer);
13966+
PyErr_NoMemory();
13967+
return NULL;
13968+
}
13969+
buffer = new_buffer;
13970+
#ifndef NDEBUG
13971+
buffer[buflen - 1] = '#';
13972+
#endif
13973+
len = strerror_r(code, buffer, buflen);
13974+
}
13975+
}
13976+
else {
13977+
buffer = small_buffer;
13978+
}
13979+
13980+
// strerror_r() always writes a trailing NUL byte
13981+
assert(memchr(buffer, 0, buflen) != NULL);
13982+
PyObject *result = PyUnicode_DecodeLocale(buffer, "surrogateescape");
13983+
if (buffer != small_buffer) {
13984+
PyMem_Free(buffer);
13985+
}
13986+
return result;
13987+
13988+
#else
13989+
// strerror() implementation
1392613990
char *message = strerror(code);
1392713991
if (message == NULL) {
1392813992
PyErr_SetString(PyExc_ValueError,
1392913993
"strerror() argument out of range");
1393013994
return NULL;
1393113995
}
1393213996
return PyUnicode_DecodeLocale(message, "surrogateescape");
13997+
#endif
1393313998
}
1393413999

1393514000

‎Python/fileutils.c‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,8 +538,14 @@ decode_current_locale(const char* arg, wchar_t **wstr, size_t *wlen,
538538
// +1 to write also the trailing NUL character
539539
size_t count = _Py_mbstowcs(res, arg, argsize + 1);
540540
if (count != DECODE_ERROR) {
541-
// Success
541+
// String decoded successfully.
542+
543+
// gh-158893: This assertion can fail if the input string was
544+
// mutated during this function call. For example, the assertion
545+
// fails on decoding strerror() result if another thread mutated
546+
// the string in-place by calling strerror() in parallel.
542547
assert(count == argsize);
548+
543549
*wstr = res;
544550
if (wlen != NULL) {
545551
*wlen = count;
@@ -694,6 +700,7 @@ _Py_DecodeLocale(const char* arg, wchar_t **wstr, size_t *wlen,
694700
assert(wstr != NULL);
695701

696702
#ifdef Py_DEBUG
703+
size_t arglen = strlen(arg);
697704
size_t wlen_canary = (size_t)-2;
698705
if (wlen) {
699706
*wlen = wlen_canary;
@@ -719,6 +726,11 @@ _Py_DecodeLocale(const char* arg, wchar_t **wstr, size_t *wlen,
719726
// Success
720727
assert(*wstr != NULL);
721728
#ifdef Py_DEBUG
729+
// gh-158893: Detect if the input string was mutated during the
730+
// function call. For example, the assertion fails on decoding
731+
// strerror() result if another thread mutated the string in-place by
732+
// calling strerror() in parallel.
733+
assert(strlen(arg) == arglen);
722734
if (wlen != NULL) {
723735
assert(*wlen == wcslen(*wstr));
724736
}

‎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.

‎configure.ac‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5559,6 +5559,9 @@ AC_CHECK_FUNCS([ \
55595559
wait wait3 wait4 waitid waitpid wcscoll wcsftime wcsxfrm wmemcmp writev \
55605560
])
55615561

5562+
PY_CHECK_FUNC_PRIVATE([strerror_r], [@%:@include <string.h>])
5563+
5564+
55625565
# os.statx uses Linux's statx function. AIX also has a function named statx,
55635566
# but it's unrelated. Check only on Linux (including Android).
55645567
AS_CASE([$ac_sys_system],

‎pyconfig.h.in‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2214,6 +2214,9 @@
22142214
/* Define if you have the 'sinpi' function. */
22152215
#undef _Py_HAVE_SINPI
22162216

2217+
/* Define if you have the 'strerror_r' function. */
2218+
#undef _Py_HAVE_STRERROR_R
2219+
22172220
/* Define if you have the 'tanpi' function. */
22182221
#undef _Py_HAVE_TANPI
22192222

0 commit comments

Comments
 (0)