Introduce process file descriptor (pidfd) based process monitoring for Linux

[iCharlesHu]

The current process monitoring code for Linux has a flaw that makes it susceptible to infinite hangs under specific conditions:

• The parent process uses any other method (other than Subprocess itself) to spawn new processes in addition to spawning with Subprocess.
• The parent process fails to properly reap the non-Subprocess-spawned process, leaving it as a zombie in the process table.

This is because currently, we rely on running waitid() with P_ALL and WNOWAIT in an infinite loop to detect possible child process state transitions. However, we don’t reap the child process (by specifying WNOWAIT) unless we (Subprocess) actually spawned the process.

Here’s a simplified pseudo-code to illustrate the issue:

while true {
    var siginfo = siginfo_t()
    // We’re not reaping the child process
    if waitid(P_ALL, id_t(0), &siginfo, WEXITED | WNOWAIT) == 0 {
        guard let c = savedContinuation else {
            // If there’s no saved continuation, we didn’t spawn the process
            // In this case, we don’t reap the child process
            continue
        }

        siginfo = siginfo_t()
        waitid(P_PID, numericCast(pid), &siginfo, WEXITED) // We’re actually reaping the child process
    }
}

With this setup, if there are zombie children in the process table without reaping, waitid(P_ALL) will repeatedly return the same (non-Subprocess-spawned) PID with every call, causing an infinite loop.

You can observe this behavior with the following sample code:

let arguments = "\"\""

let pid = arguments.withCString { args in
    var pid: pid_t = -1
    let status = posix_spawn(&pid, "/bin/echo", nil, nil, [strdup(args)] + [nil], environ)
    guard status == 0 else {
        fatalError("posix_spawn: \(status), errno: \(errno)")
    }
    return pid
}
print("echo pid: \(pid)")

let result = try await Subprocess.run(
    .path("/bin/cat"),
    arguments: ["Package.swift"],
    output: .string(limit: .max, encoding: UTF8.self),
    error: .discarded
)
print("cat finished: \(result.terminationStatus)")
print("cat output: \(result.standardOutput ?? "")")

After running this example, you’ll notice that the parent process seems to be stuck, and the “cat finished” message is never printed. This is because the parent process never calls waitid on the echo call, leaving it in the process table. Consequently, the monitor thread runs in an infinite loop.

While some may argue that this is not a bug in Subprocess, but rather an issue with the parent code, since the POSIX standard mandates that the process spawning child process must reap the child process via waitid. However, Subprocess should still not hang due to someone else’s bug.

To resolve this issue, switch to a Linux-specific process monitoring method by creating and observing the process file descriptor (pidfd) using epoll. This approach is similar to the epoll implementation introduced in #117, with the only difference being that we’re polling pidfd instead of a regular file descriptor.

As part of this change, I also unified the “process handle” design to make it easier to expose process handles to clients later (after the 1.0 release, as requested by #101). We chose to use ProcessIdentifier to host platform-specific process file descriptors and process handles because it perfectly aligns with the original use case. To ensure flexibility, we opted for a concrete ProcessIdentifier type instead of just a number, allowing us to add more information if necessary.

[iCharlesHu]

Resolves #111

[iCharlesHu]

waitid with P_PIDFD was introduced in Linux kernel 5.4, which focal should have. I'm looking into what's missing

[grynspan]

This is not a bug in the existing implementation. It is a bug in the POSIX specification (and a bug in the program.)

[jakepetroules]

Could we use clone + CLONE_PIDFD (Linux 5.2) instead of fork + pidfd_open? Like FreeBSD's pdfork, this avoids races since combining the latter two functions is not atomic.

[iCharlesHu]

Good call. Unfortunately it still needs to be conditionalized due to kernel version requirements.

[jakepetroules]

This is not a bug in the existing implementation. It is a bug in the POSIX specification (and a bug in the program.)

That may be true, but the implementation Charles is proposing here is more defensive against other parts of the program misbehaving, which seems like a good thing.

Including scenarios where zombies are being reaped correctly throughout the entire program, but maybe the body of one particular Subprocess.run call is stuck or otherwise taking an incredibly long time -- this implementation prevents that one process from holding up everything else.

[grynspan]

This will emit a deprecation warning as of *OS 26 since the standardized version has been added.

[grynspan]

Could you make this type move-only and incorporate the close() operation into deinit?

[FranzBusch]

Closing might involve closing FDs right? Which might be an asynchronous and throwing operation.

[grynspan]

That close() can fail at all is an unfortunate weird corner of POSIX that I personally tend to ignore, because a failure in close() other than EINTR/EAGAIN is basically non-recoverable. What are you even supposed to do? What can a user do to fix the problem? Generally nothing.

So I just about always just drop a close() failure on the floor. </hottake>

(As for asynchronous, it's a blocking operation in userland but it can't fail to make forward progress in this case because there's no network I/O involved unless we're doing something really wonky.)

[itingliu]

We've had this discussion in another place (here?). We ended up not calling close, but asserting that close has already been called in deinit.

But it does seem like a design people are going to reach for repeatedly. I wonder if we can put our thought process down somewhere.

[FranzBusch]

That close() can fail at all is an unfortunate weird corner of POSIX that I personally tend to ignore, because a failure in close() other than EINTR/EAGAIN is basically non-recoverable. What are you even supposed to do? What can a user do to fix the problem? Generally nothing.

I agree that it is a weirdness; nevertheless, we need to handle it and most likely surface it to the user. We shouldn't just swallow those errors.

(As for asynchronous, it's a blocking operation in userland but it can't fail to make forward progress in this case because there's no network I/O involved unless we're doing something really wonky.)

This is not entirely true. If you are using io_uring you can asynchronously listen for the subprocess termination with pidfd and signalfd via io_uring. We need to account for changes in the underlying I/O system where closing can become asynchronous otherwise we will lock ourselves in a corner API-wise.

The only pattern that keeps us flexible is a with-style based approach.

[jakepetroules]

ProcessIdentifier is only ever provided as a parameter to a closure called within a with-style function, except for runDetached. If we remove the latter from the API, there's no place where ProcessIdentifier needs to be used as a handle that needs to be closed and therefore no reason it needs to be responsible for lifetime management.

[iCharlesHu]

Added fallback implementation that uses signal handler on Linux 5.4 and below.

[iCharlesHu]

Also pin swift-system to 1.5.0 since 1.6.0 breaks on focal.

[jakepetroules]

can we just check if the pidfd is > 0 instead of doing a version check?

[iCharlesHu]

I think here we can prob whether waitid with P_PIDFD is supported, similar to the clone3 prob

[jakepetroules]

One last thing:

On Linux we have processFileDescriptor which is a PlatformFileDescriptor (CInt). On Windows we have processHandle which is also a PlatformFileDescriptor (HANDLE).

Since on both platforms the PlatformIdentifier contains a field typed as PlatformFileDescriptor, and Windows HANDLEs are largely the conceptual equivalent of Unix file descriptors, could we please give them the same name across platforms? Either rename Linux's processFileDescriptor to processHandle or rename Windows's processHandle to processFileDescriptor (and probably threadHandle to threadFileDescriptor for consistency).

[grynspan]

Windows has file descriptors too though.

[jakepetroules]

We have a child pid though. I think we need to call waitid here before returning errno?

[jakepetroules]

Windows has file descriptors too though.

True, but not for processes, and I don't think they ever would, because a HANDLE already provides all the same benefits. But maybe you're right that it's best to leave them separate.