TerminalIO: Fix ebadf on close (#719)

The stdout and stdin IOPairs in TerminalIO share the same Terminal fd.
Closing stdin first would close the Terminal, causing stdout to read -1
from fileDescriptor and fail to unregister from epoll. This left stale
handler closures (capturing the IOPair) in ProcessSupervisor.handlers
indefinitely.

Additionally, vsock sockets created in TerminalIO.start() with
closeOnDeinit disabled were never closed if the process exited before
attach() created IOPairs.

Author: dcantah

vminitd/Sources/VminitdCore/IOCloser.swift

@@ -19,3 +19,15 @@ protocol IOCloser: Sendable {
 
     func close() throws
 }
+
+struct UnownedIOCloser: IOCloser {
+    private let inner: IOCloser
+
+    var fileDescriptor: Int32 { inner.fileDescriptor }
+
+    init(_ inner: IOCloser) {
+        self.inner = inner
+    }
+
+    func close() throws {}
+}

vminitd/Sources/VminitdCore/IOPair.swift

@@ -30,6 +30,7 @@ final class IOPair: Sendable {
         let to: IOCloser
         let buffer: UnsafeMutableBufferPointer<UInt8>
         var closed: Bool
+        var registeredFd: Int32?
 
         func drain() {
             let readFrom = OSFile(fd: from.fileDescriptor)
@@ -67,11 +68,13 @@ final class IOPair: Sendable {
             self.drain()
 
             // Remove the fd from our global epoll instance first.
-            let readFromFd = self.from.fileDescriptor
-            do {
-                try ProcessSupervisor.default.unregisterFd(readFromFd)
-            } catch {
-                logger?.error("failed to delete fd from epoll \(readFromFd): \(error)")
+            if let fd = self.registeredFd {
+                do {
+                    try ProcessSupervisor.default.unregisterFd(fd)
+                } catch {
+                    logger?.error("failed to delete fd from epoll \(fd): \(error)")
+                }
+                self.registeredFd = nil
             }
 
             do {
@@ -102,7 +105,8 @@ final class IOPair: Sendable {
                 from: readFrom,
                 to: writeTo,
                 buffer: buffer,
-                closed: false
+                closed: false,
+                registeredFd: nil
             ))
         self.reason = reason
         self.logger = logger
@@ -112,7 +116,8 @@ final class IOPair: Sendable {
         self.logger?.info("setting up relay for \(reason)")
 
         let (readFromFd, writeToFd) = self.io.withLock { io in
-            (io.from.fileDescriptor, io.to.fileDescriptor)
+            io.registeredFd = io.from.fileDescriptor
+            return (io.from.fileDescriptor, io.to.fileDescriptor)
         }
 
         let readFrom = OSFile(fd: readFromFd)

vminitd/Sources/VminitdCore/TerminalIO.swift

@@ -98,7 +98,7 @@ final class TerminalIO: ManagedProcess.IO & Sendable {
             if let stdinSocket = $0.stdinSocket {
                 let pair = IOPair(
                     readFrom: stdinSocket,
-                    writeTo: term,
+                    writeTo: UnownedIOCloser(term),
                     reason: "TerminalIO stdin",
                     logger: log
                 )
@@ -121,14 +121,32 @@ final class TerminalIO: ManagedProcess.IO & Sendable {
 
     func close() throws {
         self.state.withLock {
+            // stdout must close before stdin because both IOPairs share the
+            // Terminal fd. stdout registered that fd with epoll (as its read
+            // source) and needs to unregister it while the fd is still valid.
+            // stdin closes the Terminal as its write destination, which would
+            // invalidate the fd before stdout can unregister.
+            if let stdout = $0.stdout {
+                stdout.close()
+                $0.stdout = nil
+            }
             if let stdin = $0.stdin {
                 stdin.close()
                 $0.stdin = nil
             }
-            if let stdout = $0.stdout {
-                stdout.close()
-                $0.stdout = nil
+
+            // If IOPairs were never created (process exited before attach),
+            // close the raw sockets directly since they have closeOnDeinit
+            // disabled.
+            if let stdinSocket = $0.stdinSocket {
+                try? stdinSocket.close()
+                $0.stdinSocket = nil
             }
+            if let stdoutSocket = $0.stdoutSocket {
+                try? stdoutSocket.close()
+                $0.stdoutSocket = nil
+            }
+
             $0.parent = nil
         }
     }