Skip to content

Commit 61a24ea

Browse files
[3.13] gh-137109: refactor warning about threads when forking (GH-141438) (GH-141614) (GH-141639)
[3.14] gh-137109: refactor warning about threads when forking (GH-141438) (GH-141614) This splits the OS API specific functionality to get the number of threads out from the fallback Python method and warning raising code itself. This way the OS APIs can be queried before we've run `os.register_at_fork(after_in_parent=...)` registered functions which themselves may (re)start threads that would otherwise be detected. This is best effort. If the OS APIs are either unavailable or fail, the warning generating code still falls back to looking at the Python threading state after the CPython interpreter world has been restarted and the after_in_parent calls have been made. The common case for most Linux and macOS environments should work today. This also lines up with the existing TODO refactoring, we may choose to expose this API to get the number of OS threads in the `os` module in the future. Note: This is a simplified backport that maintains the void return type for warn_about_fork_with_threads() and keeps PyErr_Clear() in the warning path, as the error handling changes from fd8f42d are not needed in 3.14. (cherry picked from commit 0d8fb0b) Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent 7beb7e8 commit 61a24ea

File tree

2 files changed

+65
-43
lines changed

2 files changed

+65
-43
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
The :mod:`os.fork` and related forking APIs will no longer warn in the
2+
common case where Linux or macOS platform APIs return the number of threads
3+
in a process and find the answer to be 1 even when a
4+
:func:`os.register_at_fork` ``after_in_parent=`` callback (re)starts a
5+
thread.

Modules/posixmodule.c

Lines changed: 60 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7880,53 +7880,19 @@ os_register_at_fork_impl(PyObject *module, PyObject *before,
78807880
// running in the process. Best effort, silent if unable to count threads.
78817881
// Constraint: Quick. Never overcounts. Never leaves an error set.
78827882
//
7883-
// This should only be called from the parent process after
7883+
// This MUST only be called from the parent process after
78847884
// PyOS_AfterFork_Parent().
78857885
static void
7886-
warn_about_fork_with_threads(const char* name)
7886+
warn_about_fork_with_threads(
7887+
const char* name, // Name of the API to use in the warning message.
7888+
const Py_ssize_t num_os_threads // Only trusted when >= 1.
7889+
)
78877890
{
78887891
// It's not safe to issue the warning while the world is stopped, because
78897892
// other threads might be holding locks that we need, which would deadlock.
78907893
assert(!_PyRuntime.stoptheworld.world_stopped);
78917894

7892-
// TODO: Consider making an `os` module API to return the current number
7893-
// of threads in the process. That'd presumably use this platform code but
7894-
// raise an error rather than using the inaccurate fallback.
7895-
Py_ssize_t num_python_threads = 0;
7896-
#if defined(__APPLE__) && defined(HAVE_GETPID)
7897-
mach_port_t macos_self = mach_task_self();
7898-
mach_port_t macos_task;
7899-
if (task_for_pid(macos_self, getpid(), &macos_task) == KERN_SUCCESS) {
7900-
thread_array_t macos_threads;
7901-
mach_msg_type_number_t macos_n_threads;
7902-
if (task_threads(macos_task, &macos_threads,
7903-
&macos_n_threads) == KERN_SUCCESS) {
7904-
num_python_threads = macos_n_threads;
7905-
}
7906-
}
7907-
#elif defined(__linux__)
7908-
// Linux /proc/self/stat 20th field is the number of threads.
7909-
FILE* proc_stat = fopen("/proc/self/stat", "r");
7910-
if (proc_stat) {
7911-
size_t n;
7912-
// Size chosen arbitrarily. ~60% more bytes than a 20th column index
7913-
// observed on the author's workstation.
7914-
char stat_line[160];
7915-
n = fread(&stat_line, 1, 159, proc_stat);
7916-
stat_line[n] = '\0';
7917-
fclose(proc_stat);
7918-
7919-
char *saveptr = NULL;
7920-
char *field = strtok_r(stat_line, " ", &saveptr);
7921-
unsigned int idx;
7922-
for (idx = 19; idx && field; --idx) {
7923-
field = strtok_r(NULL, " ", &saveptr);
7924-
}
7925-
if (idx == 0 && field) { // found the 20th field
7926-
num_python_threads = atoi(field); // 0 on error
7927-
}
7928-
}
7929-
#endif
7895+
Py_ssize_t num_python_threads = num_os_threads;
79307896
if (num_python_threads <= 0) {
79317897
// Fall back to just the number our threading module knows about.
79327898
// An incomplete view of the world, but better than nothing.
@@ -7979,6 +7945,51 @@ warn_about_fork_with_threads(const char* name)
79797945
PyErr_Clear();
79807946
}
79817947
}
7948+
7949+
// If this returns <= 0, we were unable to successfully use any OS APIs.
7950+
// Returns a positive number of threads otherwise.
7951+
static Py_ssize_t get_number_of_os_threads(void)
7952+
{
7953+
// TODO: Consider making an `os` module API to return the current number
7954+
// of threads in the process. That'd presumably use this platform code but
7955+
// raise an error rather than using the inaccurate fallback.
7956+
Py_ssize_t num_python_threads = 0;
7957+
#if defined(__APPLE__) && defined(HAVE_GETPID)
7958+
mach_port_t macos_self = mach_task_self();
7959+
mach_port_t macos_task;
7960+
if (task_for_pid(macos_self, getpid(), &macos_task) == KERN_SUCCESS) {
7961+
thread_array_t macos_threads;
7962+
mach_msg_type_number_t macos_n_threads;
7963+
if (task_threads(macos_task, &macos_threads,
7964+
&macos_n_threads) == KERN_SUCCESS) {
7965+
num_python_threads = macos_n_threads;
7966+
}
7967+
}
7968+
#elif defined(__linux__)
7969+
// Linux /proc/self/stat 20th field is the number of threads.
7970+
FILE* proc_stat = fopen("/proc/self/stat", "r");
7971+
if (proc_stat) {
7972+
size_t n;
7973+
// Size chosen arbitrarily. ~60% more bytes than a 20th column index
7974+
// observed on the author's workstation.
7975+
char stat_line[160];
7976+
n = fread(&stat_line, 1, 159, proc_stat);
7977+
stat_line[n] = '\0';
7978+
fclose(proc_stat);
7979+
7980+
char *saveptr = NULL;
7981+
char *field = strtok_r(stat_line, " ", &saveptr);
7982+
unsigned int idx;
7983+
for (idx = 19; idx && field; --idx) {
7984+
field = strtok_r(NULL, " ", &saveptr);
7985+
}
7986+
if (idx == 0 && field) { // found the 20th field
7987+
num_python_threads = atoi(field); // 0 on error
7988+
}
7989+
}
7990+
#endif
7991+
return num_python_threads;
7992+
}
79827993
#endif // HAVE_FORK1 || HAVE_FORKPTY || HAVE_FORK
79837994

79847995
#ifdef HAVE_FORK1
@@ -8013,10 +8024,12 @@ os_fork1_impl(PyObject *module)
80138024
/* child: this clobbers and resets the import lock. */
80148025
PyOS_AfterFork_Child();
80158026
} else {
8027+
// Called before AfterFork_Parent in case those hooks start threads.
8028+
Py_ssize_t num_os_threads = get_number_of_os_threads();
80168029
/* parent: release the import lock. */
80178030
PyOS_AfterFork_Parent();
80188031
// After PyOS_AfterFork_Parent() starts the world to avoid deadlock.
8019-
warn_about_fork_with_threads("fork1");
8032+
warn_about_fork_with_threads("fork1", num_os_threads);
80208033
}
80218034
if (pid == -1) {
80228035
errno = saved_errno;
@@ -8062,10 +8075,12 @@ os_fork_impl(PyObject *module)
80628075
/* child: this clobbers and resets the import lock. */
80638076
PyOS_AfterFork_Child();
80648077
} else {
8078+
// Called before AfterFork_Parent in case those hooks start threads.
8079+
Py_ssize_t num_os_threads = get_number_of_os_threads();
80658080
/* parent: release the import lock. */
80668081
PyOS_AfterFork_Parent();
80678082
// After PyOS_AfterFork_Parent() starts the world to avoid deadlock.
8068-
warn_about_fork_with_threads("fork");
8083+
warn_about_fork_with_threads("fork", num_os_threads);
80698084
}
80708085
if (pid == -1) {
80718086
errno = saved_errno;
@@ -8919,10 +8934,12 @@ os_forkpty_impl(PyObject *module)
89198934
/* child: this clobbers and resets the import lock. */
89208935
PyOS_AfterFork_Child();
89218936
} else {
8937+
// Called before AfterFork_Parent in case those hooks start threads.
8938+
Py_ssize_t num_os_threads = get_number_of_os_threads();
89228939
/* parent: release the import lock. */
89238940
PyOS_AfterFork_Parent();
89248941
// After PyOS_AfterFork_Parent() starts the world to avoid deadlock.
8925-
warn_about_fork_with_threads("forkpty");
8942+
warn_about_fork_with_threads("forkpty", num_os_threads);
89268943
}
89278944
if (pid == -1) {
89288945
return posix_error();

0 commit comments

Comments
 (0)