Prob whether waitid supports P_PIDFD instead of checking for Linux kernel version

Author: iCharlesHu

Sources/Subprocess/Platforms/Subprocess+Linux.swift

@@ -304,7 +304,7 @@ internal func monitorProcessTermination(
                 // pidfd is only supported on Linux kernel 5.4 and above
                 // On older releases, use signalfd so we do not need
                 // to register anything with epoll
-                if _isLinuxKernelAtLeast(major: 5, minor: 4) {
+                if processIdentifier.processFileDescriptor > 0 {
                     // Register processFileDescriptor with epoll
                     var event = epoll_event(
                         events: EPOLLIN.rawValue,
@@ -418,8 +418,10 @@ private extension siginfo_t {
     }
 }
 
-// Okay to be unlocked global mutable because this thread is only created once
+// Okay to be unlocked global mutable because this value is only set once like dispatch_once
 private nonisolated(unsafe) var _signalPipe: (readEnd: CInt, writeEnd: CInt) = (readEnd: -1, writeEnd: -1)
+// Okay to be unlocked global mutable because this value is only set once like dispatch_once
+private nonisolated(unsafe) var _waitProcessFileDescriptorSupported = false
 private let _processMonitorState: Mutex<ProcessMonitorState> = .init(.notStarted)
 
 private func shutdown() {
@@ -518,7 +520,7 @@ private func monitorThreadFunc(args: UnsafeMutableRawPointer?) -> UnsafeMutableR
             }
 
             // P_PIDFD requires Linux Kernel 5.4 and above
-            if _isLinuxKernelAtLeast(major: 5, minor: 4) {
+            if _waitProcessFileDescriptorSupported {
                 _blockAndWaitForProcessFileDescriptor(targetFileDescriptor, context: context)
             } else {
                 _reapAllKnownChildProcesses(targetFileDescriptor, context: context)
@@ -569,9 +571,9 @@ private let setup: () = {
         return
     }
 
-    // On Linux kernel lower than 5.4 we have to rely on signal handler
+    // If the current kernel does not support pidfd, fallback to signal handler
     // Create the self-pipe that signal handler writes to
-    if !_isLinuxKernelAtLeast(major: 5, minor: 4) {
+    if !_isWaitProcessFileDescriptorSupported() {
         var pipeCreationError: SubprocessError? = nil
         do {
             let (readEnd, writeEnd) = try FileDescriptor.pipe()
@@ -608,6 +610,9 @@ private let setup: () = {
             _reportFailureWithErrno(errno)
             return
         }
+    } else {
+        // Mark waitid(P_PIDFD) as supported
+        _waitProcessFileDescriptorSupported = true
     }
     let monitorThreadContext = MonitorThreadContext(
         epollFileDescriptor: epollFileDescriptor,
@@ -755,36 +760,25 @@ private func _reapAllKnownChildProcesses(_ signalFd: CInt, context: MonitorThrea
     }
 }
 
-private func _isLinuxKernelAtLeast(major: Int, minor: Int) -> Bool {
-    var buffer = utsname()
-    guard uname(&buffer) == 0 else {
-        return false
-    }
-    let releaseString = withUnsafeBytes(of: &buffer.release) { ptr in
-        let buffer = ptr.bindMemory(to: CChar.self)
-        return String(cString: buffer.baseAddress!)
-    }
-
-    // utsname release follows the format
-    // major.minor.patch-extra
-    guard let mainVersion = releaseString.split(separator: "-").first else {
-        return false
-    }
-
-    let versionComponents = mainVersion.split(separator: ".")
-
-    guard let currentMajor = Int(versionComponents[0]),
-          let currentMinor = Int(versionComponents[1]) else {
-        return false
-    }
-
-    if currentMajor > major {
-        return true
-    } else if currentMajor == major {
-        return currentMinor >= minor
-    } else {
+internal func _isWaitProcessFileDescriptorSupported() -> Bool {
+    // waitid(P_PIDFD) is only supported on Linux kernel 5.4 and above
+    // Prob whether the current system supports it by calling it with self pidfd
+    // and checking for EINVAL (waitid sets errno to EINVAL if it does not
+    // recognize the id type).
+    var siginfo = siginfo_t()
+    let selfPidfd = _pidfd_open(getpid())
+    if selfPidfd < 0 {
+        // If we can not retrieve pidfd, the system does not support waitid(P_PIDFD)
         return false
     }
+    /// The following call will fail either with
+    /// - ECHILD: in this case we know P_PIDFD is supported and waitid correctly
+    ///     reported that we don't have a child with the same selfPidfd;
+    /// - EINVAL: in this case we know P_PIDFD is not supported because it does not
+    ///     recognize the `P_PIDFD` type
+    waitid(idtype_t(UInt32(P_PIDFD)), id_t(selfPidfd), &siginfo, WEXITED | WNOWAIT)
+    return errno == ECHILD
 }
 
+
 #endif  // canImport(Glibc) || canImport(Android) || canImport(Musl)

Sources/_SubprocessCShims/include/process_shims.h

@@ -82,10 +82,11 @@ int _shims_snprintf(
     char * _Nonnull str2
 );
 
+int _pidfd_open(pid_t pid);
 int _pidfd_send_signal(int pidfd, int signal);
 
 // P_PIDFD is only defined on Linux Kernel 5.4 and above
-// Define our dummy value if it's not available
+// Define our value if it's not available
 #ifndef P_PIDFD
 #define P_PIDFD 3
 #endif

Sources/_SubprocessCShims/process_shims.c

@@ -311,7 +311,7 @@ int _subprocess_spawn(
 #define __GLIBC_PREREQ(maj, min) 0
 #endif
 
-static int _pidfd_open(pid_t pid) {
+int _pidfd_open(pid_t pid) {
     return syscall(SYS_pidfd_open, pid, 0);
 }
 
@@ -336,25 +336,6 @@ static int _clone3(int *pidfd) {
     return syscall(SYS_clone3, &args, sizeof(args));
 }
 
-static int _linux_kernel_version_at_least(
-    int required_major,
-    int required_minor
-) {
-    struct utsname buf;
-    if (uname(&buf) != 0) {
-        return 0;  // Cannot determine kernel version
-    }
-
-    int major = 0, minor = 0;
-    if (sscanf(buf.release, "%d.%d", &major, &minor) < 2) {
-        return 0;
-    }
-
-    if (major > required_major) return 1;
-    if (major == required_major && minor >= required_minor) return 1;
-    return 0;
-}
-
 int _subprocess_fork_exec(
     pid_t * _Nonnull pid,
     int * _Nonnull pidfd,
@@ -423,17 +404,21 @@ int _subprocess_fork_exec(
 
     // Finally, fork / clone
     int _pidfd = -1;
-    pid_t childPid = -1;
-    if (_linux_kernel_version_at_least(5, 3)) {
-        // One Linux 5.3 and above, use the new clone3 syscall
-        // So we can atomically retrieve pidfd
-        childPid = _clone3(&_pidfd);
-    } else {
-        // On older linux versions, fall back to fork()
+    // First attempt to use clone3, only fall back to fork if clone3 is not available
+    pid_t childPid = _clone3(&_pidfd);
+    if (childPid < 0) {
+        if (errno == ENOSYS) {
+            // clone3 is not implemented. Use fork instead
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wdeprecated"
-        childPid = fork();
+            childPid = fork();
 #pragma GCC diagnostic pop
+        } else {
+            // Report all other errors
+            close(pipefd[0]);
+            close(pipefd[1]);
+            return errno;
+        }
     }
 
     if (childPid < 0) {
@@ -557,7 +542,7 @@ int _subprocess_fork_exec(
     } else {
         // On Linux 5.3 and lower, we have to fetch pidfd separately
         // Newer Linux supports clone3 which returns pidfd directly
-        if (_linux_kernel_version_at_least(5, 3) == 0) {
+        if (_pidfd < 0) {
             _pidfd = _pidfd_open(childPid);
             if (_pidfd < 0) {
                 return errno;

Tests/SubprocessTests/SubprocessTests+Windows.swift

@@ -453,9 +453,10 @@ extension SubprocessWindowsTests {
 
 // MARK: - PlatformOption Tests
 extension SubprocessWindowsTests {
-    // Disabled until we investigate CI specific failures
-    // https://github.com/swiftlang/swift-subprocess/issues/128
-    @Test(.enabled(if: false))
+    @Test(
+        .disabled("Disabled until we investigate CI specific failures"),
+        .bug("https://github.com/swiftlang/swift-subprocess/issues/128")
+    )
     func testPlatformOptionsRunAsUser() async throws {
         try await self.withTemporaryUser { username, password, succeed in
             // Use public directory as working directory so the newly created user