Skip to content
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
d97417d
fix threadmodule ascii and make test more lenient
jadonduff Aug 21, 2025
1db08a7
📜🤖 Added by blurb_it.
blurb-it[bot] Aug 21, 2025
0606968
implement requested changes
jadonduff Aug 22, 2025
b556774
reformat and remove test news
jadonduff Aug 22, 2025
38a75d3
patch for windows & non-posix compliant platforms
jadonduff Aug 22, 2025
31731b1
patch
jadonduff Aug 22, 2025
83fe205
Check if generated files are up to date patch
jadonduff Aug 22, 2025
aadf7f3
test
jadonduff Aug 22, 2025
612a0a4
test2
jadonduff Aug 22, 2025
1fa51f8
test3
jadonduff Aug 22, 2025
d24d0bb
attempt fix
jadonduff Aug 22, 2025
95d289f
Merge branch 'main' into thread_name_fix
jadonduff Aug 22, 2025
d7a47bf
test
jadonduff Aug 22, 2025
1970a00
fix stray newline
jadonduff Aug 22, 2025
6395323
Merge branch 'main' into thread_name_fix
jadonduff Aug 22, 2025
241e097
Apply suggestions from code review
jadonduff Aug 22, 2025
66c058b
fixes
jadonduff Aug 22, 2025
f817412
apply fixes (built)
jadonduff Aug 22, 2025
3349464
remove dead code, fix err var, revert newline, fix comment
jadonduff Aug 23, 2025
ac3d3dd
redundant err var replaced
jadonduff Aug 23, 2025
1ab9ecd
replaced functions with inline code, moved into set_thread_name_with_…
jadonduff Aug 23, 2025
d9ba9e8
revert
jadonduff Aug 23, 2025
d7025f5
inline test
jadonduff Aug 23, 2025
3689044
test fix for "Check if generated files are up to date"
jadonduff Aug 23, 2025
246f880
fix linker error
jadonduff Aug 23, 2025
407b1e9
make regen
jadonduff Aug 23, 2025
dc43fc4
move functions and add inline for functions used once
jadonduff Aug 24, 2025
7aa256c
remove duplicated __sun enforcing utf-8 from encoder
jadonduff Aug 24, 2025
b4934a2
consolidate functions, remove unrelated changes
jadonduff Aug 24, 2025
ae5ac4b
clinic regen fix
jadonduff Aug 24, 2025
4fc368d
fix error/fallback (implement changes)
jadonduff Aug 24, 2025
392aa5a
fix whitespace lint issue in test_threading.py
jadonduff Aug 24, 2025
84daea4
Merge branch 'main' into thread_name_fix
jadonduff Aug 25, 2025
d5e9399
make _threadmodule.c clearer, remove unrelated change, add specific t…
jadonduff Aug 25, 2025
105d4e6
Merge branch 'main' into thread_name_fix
jadonduff Aug 26, 2025
c67e93d
Merge branch 'main' into thread_name_fix
jadonduff Aug 27, 2025
508f24d
add test for main and encode ascii
jadonduff Sep 2, 2025
25085bf
encode ascii on solaris/openindiana
jadonduff Sep 2, 2025
cd26833
Merge branch 'main' into thread_name_fix
jadonduff Sep 2, 2025
dc86a29
test_threading main name fixed, minor changes, news
jadonduff Sep 2, 2025
c9f554d
Merge branch 'main' into thread_name_fix
jadonduff Sep 2, 2025
c0f672e
Lint fix
jadonduff Sep 2, 2025
69796ff
Update replace to surrogateescape
jadonduff Sep 2, 2025
15c8481
fix comment length
jadonduff Sep 2, 2025
99f13ec
Merge branch 'main' into thread_name_fix
jadonduff Sep 2, 2025
8fc613b
fix comments and move thread name reset in test
jadonduff Sep 2, 2025
5e5effb
revert name reset to original position
jadonduff Sep 2, 2025
9690eee
Merge branch 'main' into thread_name_fix
jadonduff Sep 2, 2025
338593c
Update Modules/_threadmodule.c
serhiy-storchaka Sep 2, 2025
e642264
Fix EOL.
serhiy-storchaka Sep 2, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Lib/test/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -2241,6 +2241,7 @@ def __init__(self, a, *, b) -> None:

with warnings.catch_warnings(record=True) as warnings_log:
CustomRLock(1, b=2)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still an unrelated change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

self.assertEqual(warnings_log, [])

class EventTests(lock_tests.EventTests):
Expand Down Expand Up @@ -2360,6 +2361,8 @@ def work():
thread = threading.Thread(target=work, name=name)
thread.start()
thread.join()
if not name.isascii() and not work_name:
self.skipTest(f"Platform does not support non-ASCII thread names: got empty name for {name!r}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now let's get back to the tests. This change would work for non-modified code, _thread.set_name() raised an OSError which was suppressed, so the name remained empty. But now it falls back to the ASCII encoding, so the name is not empty, but it is not an original name either. So this no longer works.

There is already special code for Solaris above. I am surprised that there is such difference in behavior between Solaris and OpenIndiana. Maybe just setting encoding = 'ascii' on OpenIndiana can fix the test. It seems that the only way to differentiate OpenIndiana from Solaris is to check if os.uname().version.startswith('illumos').

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What Solaris/OpenIndiana difference do you mean? I am pretty sure that if sys.platform.startswith("sunos") is true on both our Solaris and OpenIndiana (though I will check just to be sure).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a specific case for openindiana/illumos in test_threading.py. However, let me know if you want this revered/change after kulikjak's comment

            with self.subTest(name=name, expected=expected):
                work_name = None
                thread = threading.Thread(target=work, name=name)
                thread.start()
                thread.join()

                # Detect if running on OpenIndiana / illumos
                try:
                    is_illumos = os.uname().version.startswith('illumos')
                except AttributeError:
                    is_illumos = False

                if is_illumos:
                    # illumos requires ASCII-encoded thread names
                    if not work_name:
                        # name didn't get set (set_name may have failed)
                        self.skipTest(
                            f"Platform does not support non-ASCII thread names: got empty name for {name!r}"
                        )

                    work_name_bytes = (
                        work_name.encode('ascii', errors='ignore')
                        if isinstance(work_name, str)
                        else work_name
                    )
                    expected_bytes = expected.encode('ascii', errors='ignore')

                    self.assertEqual(
                        work_name_bytes,
                        expected_bytes,
                        f"{len(work_name)=} and {len(expected)=}"
                    )

                elif not name.isascii() and not work_name:
                    # Platform does not support non-ASCII thread names
                    self.skipTest(
                        f"Platform does not support non-ASCII thread names: got empty name for {name!r}"
                    )

                else:
                    # Most platforms
                    self.assertEqual(work_name, expected,
                                    f"{len(work_name)=} and {len(expected)=}")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What Solaris/OpenIndiana difference do you mean?

_thread.set_name() does not work for non-ASCII names on OpenIndiana. But it works on Solaris, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, I am sorry - I misunderstood what you meant. Yes, Oracle Solaris does work with non-ASCII names - here tested with latest main:

> LD_LIBRARY_PATH=./build/prototype/sparc/usr/lib/sparcv9 ./build/sparcv9/python
Python 3.15.0a0 (main, Aug 26 2025, 11:02:43) [GCC 14.2.0] on sunos5
Type "help", "copyright", "credits" or "license" for more information.
>>> import _thread
>>> _thread.set_name('€')
>>>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I just re-tested the change and confirmed that the tests indeed go through the # Most platforms branch.

self.assertEqual(work_name, expected,
f"{len(work_name)=} and {len(expected)=}")

Expand Down
1 change: 1 addition & 0 deletions Misc/ACKS
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ Weilin Du
John DuBois
Paul Dubois
Jacques Ducasse
Jadon Duff
Andrei Dorian Duma
Graham Dumpleton
Quinn Dunkan
Expand Down
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.
105 changes: 73 additions & 32 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
#ifdef HAVE_SIGNAL_H
# include <signal.h> // SIGINT
#endif
#ifdef HAVE_PTHREAD_H
# include <pthread.h>
#endif
#include <errno.h>
#include <string.h>

// ThreadError is just an alias to PyExc_RuntimeError
#define ThreadError PyExc_RuntimeError
Expand Down Expand Up @@ -2517,6 +2522,58 @@ of the main interpreter.");
# include <pthread_np.h>
#endif

/* Helper: encode/truncate and call native API to set thread name.
* Return:
* 0 : success
* >0 : errno-style native error code (e.g. EINVAL)
* -1 : Python-level error (an exception has been set)
*/
static int
set_encoded_thread_name(PyObject *name_obj, const char *encoding)
{
PyObject *name_encoded = PyUnicode_AsEncodedString(name_obj, encoding, "replace");
if (name_encoded == NULL) {
/* PyUnicode_AsEncodedString set an exception */
return -1;
}

#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);
Py_DECREF(name_encoded);
if (truncated == NULL) {
/* PyBytes_FromStringAndSize set an exception */
return -1;
}
name_encoded = truncated;
}
#endif

const char *name = PyBytes_AS_STRING(name_encoded);
int rc = 0;

#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);
#elif defined(HAVE_PTHREAD_SET_NAME_NP)
pthread_t thread = pthread_self();
pthread_set_name_np(thread, name);
rc = 0; /* that API returns void */
#else
rc = 0; /* no-op if platform unsupported */
#endif

Py_DECREF(name_encoded);
return rc;
}

#if defined(HAVE_PTHREAD_GETNAME_NP) || defined(HAVE_PTHREAD_GET_NAME_NP) || defined(MS_WINDOWS)
/*[clinic input]
_thread._get_name
Expand Down Expand Up @@ -2590,42 +2647,26 @@ _thread_set_name_impl(PyObject *module, PyObject *name_obj)
PyInterpreterState *interp = _PyInterpreterState_GET();
const char *encoding = interp->unicode.fs_codec.encoding;
#endif
PyObject *name_encoded;
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);
if (truncated == NULL) {
Py_DECREF(name_encoded);
return NULL;
}
Py_SETREF(name_encoded, truncated);
int rc = set_encoded_thread_name(name_obj, encoding);
/* Confirm a Python exception was set by the helper.
If not, convert to a runtime error (defensive). */
if (rc == -1 && PyErr_Occurred()) {
return NULL;
}
#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
Py_DECREF(name_encoded);
if (rc) {
/* If native API refused (EINVAL) and we didn't try ASCII, retry with ASCII. */
if (rc == EINVAL && strcmp(encoding, "ascii") != 0) {
rc = set_encoded_thread_name(name_obj, "ascii");
if (rc == -1 && PyErr_Occurred()) {
return NULL;
}
if (rc == 0) {
Py_RETURN_NONE;
}
/* fall through to raise errno below */
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more nitpicks.

Move this code outside of the if (rc) {...} block, so if (rc) {...} will only contain the code that raises OSError. Then remove if (rc == 0) {...} -- it will no longer be needed.

I believe the code will be a little clearer after that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Fixed

errno = rc;
return PyErr_SetFromErrno(PyExc_OSError);
}
Expand Down
Loading