Merge pull request swiftlang#81457 from mikeash/swift-inspect-corpse-leak

[swift-inspect] Fix corpse leaks when target doesn't have libswiftCore loaded.

Author: mikeash

tools/swift-inspect/Sources/swift-inspect/Cleanup.swift

@@ -0,0 +1,48 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+/// A helper struct that manages the cleanup of some external resource.
+/// The value is stored as an Optional, but presented as a non-optional.
+/// Accessing the value before one has been set is a runtime error. The cleanup
+/// function is called on the value if one has been set, when setting a new
+/// value or when the struct is destroyed.
+struct Cleanup<T>: ~Copyable {
+  /// The underlying Optional storage for the value.
+  private var _value: T?
+
+  /// The wrapped value. A value must be set before any value is read here.
+  var value: T {
+    get {
+      _value!
+    }
+    set {
+      if let _value {
+        cleanup(_value)
+      }
+      _value = newValue
+    }
+  }
+
+  /// The function used to clean up the resource held by this struct.
+  let cleanup: (T) -> Void
+
+  init(cleanup: @escaping (T) -> Void) {
+    self._value = nil
+    self.cleanup = cleanup
+  }
+
+  deinit {
+    if let _value {
+      cleanup(_value)
+    }
+  }
+}

tools/swift-inspect/Sources/swift-inspect/DarwinRemoteProcess.swift

@@ -19,13 +19,25 @@ internal final class DarwinRemoteProcess: RemoteProcess {
   public typealias ProcessIdentifier = pid_t
   public typealias ProcessHandle = task_t
 
-  private var task: task_t
+  private var task = Cleanup<task_t> {
+    // task_stop_peeking does nothing if we didn't task_start_peeking first, so
+    // we can call it unconditionally.
+    task_stop_peeking($0)
+    mach_port_deallocate(mach_task_self_, $0)
+  }
+
   internal var processIdentifier: ProcessIdentifier
   internal lazy var processName = getProcessName(processId: processIdentifier) ?? "<unknown process>"
 
-  public var process: ProcessHandle { task }
-  public private(set) var context: SwiftReflectionContextRef!
-  private var symbolicator: CSSymbolicatorRef
+  public var process: ProcessHandle { task.value }
+  private var _context = Cleanup<SwiftReflectionContextRef> {
+    swift_reflection_destroyReflectionContext($0)
+  }
+  public var context: SwiftReflectionContextRef! { _context.value }
+
+  private var symbolicator = Cleanup<CSSymbolicatorRef> {
+    CSRelease($0)
+  }
 
   private var swiftCore: CSTypeRef
   private let swiftConcurrency: CSTypeRef
@@ -72,7 +84,7 @@ internal final class DarwinRemoteProcess: RemoteProcess {
   }
 
   func read(address: swift_addr_t, size: Int) -> UnsafeRawPointer? {
-    return task_peek(task, address, mach_vm_size_t(size))
+    return task_peek(task.value, address, mach_vm_size_t(size))
   }
 
   func getAddr(symbolName: String) -> swift_addr_t {
@@ -98,7 +110,7 @@ internal final class DarwinRemoteProcess: RemoteProcess {
   static var GetStringLength: GetStringLengthFunction {
     return { (context, address) in
       let process: DarwinRemoteProcess = DarwinRemoteProcess.fromOpaque(context!)
-      if let str = task_peek_string(process.task, address) {
+      if let str = task_peek_string(process.task.value, address) {
         return UInt64(strlen(str))
       }
       return 0
@@ -119,18 +131,19 @@ internal final class DarwinRemoteProcess: RemoteProcess {
 
   init?(processId: ProcessIdentifier, forkCorpse: Bool) {
     processIdentifier = processId
-    var task: task_t = task_t()
-    let taskResult = task_for_pid(mach_task_self_, processId, &task)
+    var processTask: task_t = task_t()
+    let taskResult = task_for_pid(mach_task_self_, processId, &processTask)
     guard taskResult == KERN_SUCCESS else {
       print("unable to get task for pid \(processId): \(String(cString: mach_error_string(taskResult))) \(hex: taskResult)",
         to: &Std.err)
       return nil
     }
+    self.task.value = processTask
 
     // Consult with VMUProcInfo to determine if we should force forkCorpse.
     let forceForkCorpse: Bool
     if let procInfoClass = getVMUProcInfoClass() {
-      let procInfo = procInfoClass.init(task: task)
+      let procInfo = procInfoClass.init(task: self.task.value)
       forceForkCorpse = procInfo.shouldAnalyzeWithCorpse
     } else {
       // Default to not forcing forkCorpse.
@@ -141,11 +154,9 @@ internal final class DarwinRemoteProcess: RemoteProcess {
       var corpse = task_t()
       let maxRetry = 6
       for retry in 0..<maxRetry {
-        let corpseResult = task_generate_corpse(task, &corpse)
+        let corpseResult = task_generate_corpse(task.value, &corpse)
         if corpseResult == KERN_SUCCESS {
-          task_stop_peeking(task)
-          mach_port_deallocate(mach_task_self_, task)
-          task = corpse
+          task.value = corpse
           break
         }
         if corpseResult != KERN_RESOURCE_SHORTAGE || retry == maxRetry {
@@ -157,20 +168,19 @@ internal final class DarwinRemoteProcess: RemoteProcess {
       }
     }
 
-    self.task = task
+    self.symbolicator.value = CSSymbolicatorCreateWithTask(self.task.value)
+
+    self.swiftCore = CSSymbolicatorGetSymbolOwnerWithNameAtTime(
+        self.symbolicator.value, "libswiftCore.dylib", kCSNow)
+    self.swiftConcurrency = CSSymbolicatorGetSymbolOwnerWithNameAtTime(
+        self.symbolicator.value, "libswift_Concurrency.dylib", kCSNow)
 
-    self.symbolicator = CSSymbolicatorCreateWithTask(self.task)
-    self.swiftCore =
-        CSSymbolicatorGetSymbolOwnerWithNameAtTime(self.symbolicator,
-                                                   "libswiftCore.dylib", kCSNow)
     if CSIsNull(self.swiftCore) {
       print("pid \(processId) does not have libswiftCore.dylib loaded")
       return nil
     }
 
-    self.swiftConcurrency = CSSymbolicatorGetSymbolOwnerWithNameAtTime(
-      symbolicator, "libswift_Concurrency.dylib", kCSNow)
-    _ = task_start_peeking(self.task)
+    _ = task_start_peeking(self.task.value)
 
     guard let context =
         swift_reflection_createReflectionContextWithDataLayout(self.toOpaqueRef(),
@@ -181,23 +191,17 @@ internal final class DarwinRemoteProcess: RemoteProcess {
                                                                Self.GetSymbolAddress) else {
       return nil
     }
-    self.context = context
+    self._context.value = context
 
-    _ = CSSymbolicatorForeachSymbolOwnerAtTime(self.symbolicator, kCSNow, { owner in
+    _ = CSSymbolicatorForeachSymbolOwnerAtTime(self.symbolicator.value, kCSNow, { owner in
       let address = CSSymbolOwnerGetBaseAddress(owner)
       _ = swift_reflection_addImage(self.context, address)
     })
   }
 
-  deinit {
-    task_stop_peeking(self.task)
-    CSRelease(self.symbolicator)
-    mach_port_deallocate(mach_task_self_, self.task)
-  }
-
   func symbolicate(_ address: swift_addr_t) -> (module: String?, symbol: String?) {
     let symbol =
-        CSSymbolicatorGetSymbolWithAddressAtTime(self.symbolicator, address, kCSNow)
+        CSSymbolicatorGetSymbolWithAddressAtTime(self.symbolicator.value, address, kCSNow)
 
     let module = CSSymbolGetSymbolOwner(symbol)
     return (CSSymbolOwnerGetName(module), CSSymbolGetName(symbol))
@@ -206,7 +210,7 @@ internal final class DarwinRemoteProcess: RemoteProcess {
   internal func iterateHeap(_ body: (swift_addr_t, UInt64) -> Void) {
     withoutActuallyEscaping(body) {
       withUnsafePointer(to: $0) {
-        task_enumerate_malloc_blocks(self.task,
+        task_enumerate_malloc_blocks(self.task.value,
                                      UnsafeMutableRawPointer(mutating: $0),
                                      CUnsignedInt(MALLOC_PTR_IN_USE_RANGE_TYPE),
                                      { (task, context, type, ranges, count) in
@@ -264,14 +268,14 @@ extension DarwinRemoteProcess {
   }
 
   private func getThreadInfos() -> [ThreadInfo] {
-    guard let threads = PortList(task: self.task) else {
+    guard let threads = PortList(task: self.task.value) else {
       return []
     }
-    return threads.compactMap {
-      guard let info = getThreadInfo(thread: $0) else {
+    return threads.compactMap { t -> ThreadInfo? in
+      guard let info = getThreadInfo(thread: t) else {
         return nil
       }
-      guard let kernelObj = getKernelObject(task: mach_task_self_, port: $0) else {
+      guard let kernelObj = getKernelObject(task: mach_task_self_, port: t) else {
         return nil
       }
       return ThreadInfo(threadID: info.thread_id,
@@ -328,7 +332,7 @@ extension DarwinRemoteProcess {
   }
 
   internal func getThreadID(remotePort: thread_t) -> UInt64? {
-    guard let remoteThreadObj = getKernelObject(task: self.task, port: remotePort) else {
+    guard let remoteThreadObj = getKernelObject(task: self.task.value, port: remotePort) else {
       return nil
     }
     return threadInfos.first{ $0.kernelObject == remoteThreadObj }?.threadID

tools/swift-inspect/Sources/swift-inspect/Process.swift

@@ -80,7 +80,7 @@ internal func getAllProcesses(options: UniversalOptions) -> [ProcessIdentifier]?
   }
   let newCount = bufferSize / kinfo_stride
   if count > newCount {
-    buffer.dropLast(count - newCount)
+    buffer.removeLast(count - newCount)
   }
   let sorted = buffer.sorted { first, second in
     first.kp_proc.p_pid > second.kp_proc.p_pid