_thread.set_name(): doubt about _PYTHREAD_NAME_MAXLEN values for BSD operating systems

[xavierog]

Bug report

FreeBSD

Bug description:

6088b37 states that "FreeBSD truncates to 98 bytes silently". This figure stems from an empirical test, but I have reasons to doubt it reflects reality and believe the matter should be either fixed or clarified.

User perspective

On FreeBSD, ps --libxo json -Ho tdname returns a maximum of 19 characters, which matches MAXCOMLEN:

$ grep MAXCOMLEN /usr/include/sys/param.h
 * MAXCOMLEN should be >= sizeof(ac_comm) (see <acct.h>)
#define MAXCOMLEN       19              /* max command name remembered */

System perspective

Let's have a look at FreeBSD's implementation of pthread_setname_np(). For the purpose of this discussion, it can be simplified down to this:

int _pthread_setname_np(pthread_t thread, const char *name) {
    /* thr_set_name(2): The name will be silently truncated to fit
       into a buffer of MAXCOMLEN + 1 bytes. */
    if (thr_set_name(thread->tid, name) == -1) /* this is a syscall */
        res = errno;
    else
        thr_set_name_np(thread, &tmp_name); /* thread->name = *tmp_name; */
}

According to thr_private.h, thread->name is a simple char *.

As to pthread_getname_np(), it boils down to:

strlcpy(buf, thread->name, len);

Otherly put, pthread_setname_np() sets the thread name through a syscall that truncates to MAXCOMLEN=19 chars and keeps an untouched copy of it. However, that untouched copy may get truncated to the length passed to pthread_getname_np(). Here, I am tempted to say that 98-char limitation actually stems from this:

cpython/Modules/_threadmodule.c

Lines 2408 to 2410 in 55815a6

	char name[100];
	pthread_t thread = pthread_self();
	int rc = pthread_getname_np(thread, name, Py_ARRAY_LENGTH(name));

Working with a 100-char array implies a 99-character string. Not sure how we get from 99 to 98, but I agree with @vstinner that:

There are 2 hard problems in computer science: cache invalidation, naming things, and off-by-1 errors.

Third perspective

Before diving into FreeBSD's libpthread implementation, I had a look at sys/procfs.h:

#define PRFNAMESZ   16  /* Maximum command length saved */
#define PRARGSZ     80  /* Maximum argument bytes saved */

#define PRPSINFO_VERSION    1   /* Current version of prpsinfo_t */

typedef struct prpsinfo {
    int     pr_version; /* Version number of struct (1) */
    size_t  pr_psinfosz;    /* sizeof(prpsinfo_t) (1) */
    char    pr_fname[PRFNAMESZ+1];  /* Command name, null terminated (1) */
    char    pr_psargs[PRARGSZ+1];   /* Arguments, null terminated (1) */
    pid_t   pr_pid;     /* Process ID (1a) */
} prpsinfo_t;

typedef struct thrmisc {
    char    pr_tname[MAXCOMLEN+1];  /* Thread name, null terminated (1) */
    u_int   _pad;           /* Convenience pad, 0-filled (1) */
} thrmisc_t;

Although struct thrmisc confirms my hunch that thread names are truncated to MAXCOMLEN=19 chars, I noticed that, within struct prpsinfo, PRFNAMESZ+1 + PRARGSZ+1 = 16+1 + 80+1 = 98. This was a little scary because that would imply null bytes conveniently appearing thanks to alignment (or similar shenanigans).

Kernel perspective

The implementation of the thr_set_name() syscall confirms the FreeBSD kernel truncates thread names to MAXCOMLEN chars (specifically, it tries to copy the entire name then handles ENAMETOOLONG).

Conclusion

Although I am not 100% sure of why we got 98 instead of 99 or 100, it seems 19 would be a better choice.

OpenBSD

Bug description:

The current implementation mentions FreeBSD and NetBSD but not OpenBSD.

System perspective

The librthread implementation of pthread_set_name_np() and pthread_get_name_np() invokes the setthrname and getthrname syscalls.

getthrname(2) says:

    setthrname() may return the following errors:

    [EINVAL]           The name argument pointed to a string that was too
                       long.  Thread names are limited to MAXCOMLEN
                       characters, currently 23.

MAXCOMLEN is indeed 23:

/usr/include/sys/param.h:#define        MAXCOMLEN       _MAXCOMLEN-1    /* max command name remembered, without NUL */
/usr/include/sys/syslimits.h:#define _MAXCOMLEN         24      /* includes NUL */

Conclusion

configure.ac should have an extra line:

OpenBSD*) _PYTHREAD_NAME_MAXLEN=23;;

NetBSD

Bug description

The current implementation:

NetBSD*) _PYTHREAD_NAME_MAXLEN=31;;

... is seemingly correct from the system (libpthread) perspective but it does not take the kernel perspective into account.

System perspective

The implementation of pthread_setname_np() returns EINVAL if snprintf() reports it had to truncate the resulting name to PTHREAD_MAX_NAMELEN_NP chars.

PTHREAD_MAX_NAMELEN_NP is 32.
/usr/include/pthread.h:#define PTHREAD_MAX_NAMELEN_NP 32

pthread_setname_np() then calls _lwp_setname(), which is a syscall.

Kernel perspective

The implementation of the _lwp_setname() syscall works with a NULL-terminated MAXCOMLEN-long string.

MAXCOMLEN is 16.
/usr/include/sys/param.h:#define MAXCOMLEN 16 /* max command name remembered */

Otherly put, thread names cannot exceed 15 characters on NetBSD.

Conclusion

The correct implementation should be:

NetBSD*) _PYTHREAD_NAME_MAXLEN=15;;

CPython versions tested on:

CPython main branch

Operating systems tested on:

Other

Linked PRs

• gh-131268: Adjust _PYTHREAD_NAME_MAXLEN on BSD #131345
• gh-131268: implement thread names on OpenBSD #131528

[vstinner]

Nice analysis. Do you want to propose a fix?

[xavierog]

Not yet. I am first going to expand this analysis to OpenBSD and NetBSD.

[xavierog]

Ok, I am done analyzing FreeBSD, OpenBSD and NetBSD. I have updated the report above to reflect my findings.

In the end, I believe the fix should be:

diff --git a/configure b/configure
index d0ae103014a..b0605f251a9 100755
--- a/configure
+++ b/configure
@@ -30451,10 +30451,11 @@ CPPFLAGS=$save_CPPFLAGS
 case "$ac_sys_system" in
   Linux*) _PYTHREAD_NAME_MAXLEN=15;;  # Linux and Android
   SunOS*) _PYTHREAD_NAME_MAXLEN=31;;
-  NetBSD*) _PYTHREAD_NAME_MAXLEN=31;;
+  NetBSD*) _PYTHREAD_NAME_MAXLEN=15;;
   Darwin) _PYTHREAD_NAME_MAXLEN=63;;
   iOS) _PYTHREAD_NAME_MAXLEN=63;;
-  FreeBSD*) _PYTHREAD_NAME_MAXLEN=98;;
+  FreeBSD*) _PYTHREAD_NAME_MAXLEN=19;;
+  OpenBSD*) _PYTHREAD_NAME_MAXLEN=23;;
   *) _PYTHREAD_NAME_MAXLEN=;;
 esac
 if test -n "$_PYTHREAD_NAME_MAXLEN"; then
diff --git a/configure.ac b/configure.ac
index 8bb0f1c6ef4..8120ff3040b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -7559,10 +7559,11 @@ _RESTORE_VAR([CPPFLAGS])
 case "$ac_sys_system" in
   Linux*) _PYTHREAD_NAME_MAXLEN=15;;  # Linux and Android
   SunOS*) _PYTHREAD_NAME_MAXLEN=31;;
-  NetBSD*) _PYTHREAD_NAME_MAXLEN=31;;
+  NetBSD*) _PYTHREAD_NAME_MAXLEN=15;;
   Darwin) _PYTHREAD_NAME_MAXLEN=63;;
   iOS) _PYTHREAD_NAME_MAXLEN=63;;
-  FreeBSD*) _PYTHREAD_NAME_MAXLEN=98;;
+  FreeBSD*) _PYTHREAD_NAME_MAXLEN=19;;
+  OpenBSD*) _PYTHREAD_NAME_MAXLEN=23;;
   *) _PYTHREAD_NAME_MAXLEN=;;
 esac
 if test -n "$_PYTHREAD_NAME_MAXLEN"; then

@vstinner What do you think?

[vstinner]

I converted your inline patch to a pull request: #131345.

[vstinner]

I updated the constant on FreeBSD and NetBSD. Thanks!

I discussed with @xavierog in private and he told me that the OpenBSD implementation doesn't work, so it's not worth it to update the OpenBSD constant.

I close the issue.

[xavierog]

About the OpenBSD implementation: as this comment is being written, thread naming happens only if the pthread_setname_np() function is available at compile-time. BUT OpenBSD does not provide this function. Instead, it provides pthread_set_name_np() (note the extra underscore).
I intend to submit a PR to deal with that subtlety in the coming days.