-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
gh-138004: fix threadmodule ascii and make thread naming test more lenient #138017
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 12 commits
d97417d
1db08a7
0606968
b556774
38a75d3
31731b1
83fe205
aadf7f3
612a0a4
1fa51f8
d24d0bb
95d289f
d7a47bf
1970a00
6395323
241e097
66c058b
f817412
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2241,6 +2241,7 @@ def __init__(self, a, *, b) -> None: | |
|
||
with warnings.catch_warnings(record=True) as warnings_log: | ||
CustomRLock(1, b=2) | ||
|
||
self.assertEqual(warnings_log, []) | ||
|
||
class EventTests(lock_tests.EventTests): | ||
|
@@ -2358,10 +2359,20 @@ def work(): | |
with self.subTest(name=name, expected=expected): | ||
work_name = None | ||
thread = threading.Thread(target=work, name=name) | ||
thread.start() | ||
thread.join() | ||
self.assertEqual(work_name, expected, | ||
f"{len(work_name)=} and {len(expected)=}") | ||
try: | ||
thread.start() | ||
thread.join() | ||
# If the name is non-ASCII and the result is empty, skip (platform limitation) | ||
if any(ord(c) > 127 for c in name) and (not work_name or work_name == ""): | ||
self.skipTest(f"Platform does not support non-ASCII thread names: got empty name for {name!r}") | ||
self.assertEqual(work_name, expected, | ||
f"{len(work_name)=} and {len(expected)=}") | ||
except OSError as exc: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is OSError even raised here? The test failure was different -- that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added:
Kept OSError fallback because it was shown in original issue, but can remove if needed.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was shown that |
||
# Accept EINVAL (22) for non-ASCII names on platforms that do not support them | ||
if getattr(exc, 'errno', None) == 22 and any(ord(c) > 127 for c in name): | ||
self.skipTest(f"Platform does not support non-ASCII thread names: {exc}") | ||
else: | ||
raise | ||
|
||
@unittest.skipUnless(hasattr(_thread, 'set_name'), "missing _thread.set_name") | ||
@unittest.skipUnless(hasattr(_thread, '_get_name'), "missing _thread._get_name") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
_thread.set_name() now retries with an ASCII fallback if pthread_setname_np() rejects UTF-8 names on some POSIX-compliant platforms. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,14 +71,22 @@ get_thread_state_by_cls(PyTypeObject *cls) | |
return get_thread_state(module); | ||
} | ||
|
||
|
||
#ifdef MS_WINDOWS | ||
typedef HRESULT (WINAPI *PF_GET_THREAD_DESCRIPTION)(HANDLE, PCWSTR*); | ||
typedef HRESULT (WINAPI *PF_SET_THREAD_DESCRIPTION)(HANDLE, PCWSTR); | ||
static PF_GET_THREAD_DESCRIPTION pGetThreadDescription = NULL; | ||
static PF_SET_THREAD_DESCRIPTION pSetThreadDescription = NULL; | ||
#endif | ||
|
||
#if defined(HAVE_PTHREAD_SETNAME_NP) || defined(HAVE_PTHREAD_SET_NAME_NP) | ||
static int _set_thread_name(const char *name); | ||
#endif | ||
|
||
// Fallback: Provides a no-op implementation if neither pthread naming API is available. This avoids linker errors and provides a portable stub. | ||
#if !defined(HAVE_PTHREAD_SETNAME_NP) && !defined(HAVE_PTHREAD_SET_NAME_NP) | ||
static int _set_thread_name(const char *name) { return 0; } | ||
#endif | ||
|
||
|
||
/*[clinic input] | ||
module _thread | ||
|
@@ -2576,55 +2584,85 @@ _thread.set_name | |
Set the name of the current thread. | ||
[clinic start generated code]*/ | ||
|
||
|
||
#ifndef MS_WINDOWS | ||
// Helper to set the thread name using platform-specific APIs (POSIX only) | ||
static int | ||
_set_thread_name(const char *name) | ||
{ | ||
int rc; | ||
#ifdef __APPLE__ | ||
rc = pthread_setname_np(name); | ||
#elif defined(__NetBSD__) | ||
pthread_t thread = pthread_self(); | ||
rc = pthread_setname_np(thread, "%s", (void *)name); | ||
#elif defined(HAVE_PTHREAD_SETNAME_NP) | ||
pthread_t thread = pthread_self(); | ||
rc = pthread_setname_np(thread, name); | ||
#else /* defined(HAVE_PTHREAD_SET_NAME_NP) */ | ||
pthread_t thread = pthread_self(); | ||
rc = 0; /* pthread_set_name_np() returns void */ | ||
pthread_set_name_np(thread, name); | ||
#endif | ||
return rc; | ||
} | ||
#endif // !MS_WINDOWS | ||
|
||
|
||
static PyObject * | ||
_thread_set_name_impl(PyObject *module, PyObject *name_obj) | ||
/*[clinic end generated code: output=402b0c68e0c0daed input=7e7acd98261be82f]*/ | ||
{ | ||
#ifndef MS_WINDOWS | ||
// POSIX and non-Windows platforms | ||
#ifdef __sun | ||
// Solaris always uses UTF-8 | ||
jadonduff marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const char *encoding = "utf-8"; | ||
#else | ||
// Encode the thread name to the filesystem encoding using the "replace" | ||
// error handler | ||
PyInterpreterState *interp = _PyInterpreterState_GET(); | ||
const char *encoding = interp->unicode.fs_codec.encoding; | ||
#endif | ||
PyObject *name_encoded; | ||
int rc; | ||
|
||
name_encoded = PyUnicode_AsEncodedString(name_obj, encoding, "replace"); | ||
if (name_encoded == NULL) { | ||
return NULL; | ||
} | ||
|
||
#ifdef _PYTHREAD_NAME_MAXLEN | ||
// Truncate to _PYTHREAD_NAME_MAXLEN bytes + the NUL byte if needed | ||
if (PyBytes_GET_SIZE(name_encoded) > _PYTHREAD_NAME_MAXLEN) { | ||
PyObject *truncated; | ||
truncated = PyBytes_FromStringAndSize(PyBytes_AS_STRING(name_encoded), | ||
_PYTHREAD_NAME_MAXLEN); | ||
PyObject *truncated = PyBytes_FromStringAndSize(PyBytes_AS_STRING(name_encoded), _PYTHREAD_NAME_MAXLEN); | ||
if (truncated == NULL) { | ||
Py_DECREF(name_encoded); | ||
return NULL; | ||
} | ||
Py_SETREF(name_encoded, truncated); | ||
} | ||
#endif | ||
|
||
const char *name = PyBytes_AS_STRING(name_encoded); | ||
#ifdef __APPLE__ | ||
int rc = pthread_setname_np(name); | ||
#elif defined(__NetBSD__) | ||
pthread_t thread = pthread_self(); | ||
int rc = pthread_setname_np(thread, "%s", (void *)name); | ||
#elif defined(HAVE_PTHREAD_SETNAME_NP) | ||
pthread_t thread = pthread_self(); | ||
int rc = pthread_setname_np(thread, name); | ||
#else /* defined(HAVE_PTHREAD_SET_NAME_NP) */ | ||
pthread_t thread = pthread_self(); | ||
int rc = 0; /* pthread_set_name_np() returns void */ | ||
pthread_set_name_np(thread, name); | ||
#endif | ||
rc = _set_thread_name(name); | ||
Py_DECREF(name_encoded); | ||
|
||
// Fallback: If EINVAL, try ASCII encoding with "replace" | ||
if (rc == EINVAL) { | ||
name_encoded = PyUnicode_AsEncodedString(name_obj, "ascii", "replace"); | ||
if (name_encoded == NULL) { | ||
return NULL; | ||
} | ||
#ifdef _PYTHREAD_NAME_MAXLEN | ||
if (PyBytes_GET_SIZE(name_encoded) > _PYTHREAD_NAME_MAXLEN) { | ||
PyObject *truncated = PyBytes_FromStringAndSize(PyBytes_AS_STRING(name_encoded), _PYTHREAD_NAME_MAXLEN); | ||
if (truncated == NULL) { | ||
Py_DECREF(name_encoded); | ||
return NULL; | ||
} | ||
Py_SETREF(name_encoded, truncated); | ||
} | ||
#endif | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, please try to avoid duplicating code. This should be factored out into its own function. Here's an outline: static PyObject *
get_truncated(PyObject *name_encoded /* stolen */)
{
#ifdef _PYTHREAD_NAME_MAXLEN
if (PyBytes_GET_SIZE(name_encoded) > _PYTHREAD_NAME_MAXLEN) {
PyObject *truncated = PyBytes_FromStringAndSize(PyBytes_AS_STRING(name_encoded), _PYTHREAD_NAME_MAXLEN);
if (truncated == NULL) {
Py_DECREF(name_encoded);
return NULL;
}
Py_SETREF(name_encoded, truncated);
}
#endif
return name_encoded;
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or simply include the encoding and truncating code in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created function encode_thread_name and used to replace duplicated code |
||
name = PyBytes_AS_STRING(name_encoded); | ||
rc = _set_thread_name(name); | ||
Py_DECREF(name_encoded); | ||
} | ||
|
||
if (rc) { | ||
errno = rc; | ||
return PyErr_SetFromErrno(PyExc_OSError); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stray newline change: