OCI blob compression: fix Data memory leak when using InputFilter (#183)

* OCI blob compression: fix Data memory leak when using InputFilter

* Rename mappedDiskOffset to mappedDiskReadOffset

* Update progress.completedUnitCount differently

.cirrus.yml

@@ -10,7 +10,7 @@ task:
   name: Build
   only_if: $CIRRUS_TAG == ''
   macos_instance:
-    image: ghcr.io/cirruslabs/macos-monterey-xcode:14
+    image: ghcr.io/cirruslabs/macos-ventura-xcode:latest
   build_script: swift build --product tart
   sign_script: codesign --sign - --entitlements Resources/tart.entitlements --force .build/debug/tart
   binary_artifacts:
@@ -20,7 +20,7 @@ task:
   name: Release
   only_if: $CIRRUS_TAG != ''
   macos_instance:
-    image: ghcr.io/cirruslabs/macos-monterey-xcode:14
+    image: ghcr.io/cirruslabs/macos-ventura-xcode:latest
   env:
     GITHUB_TOKEN: ENCRYPTED[!98ace8259c6024da912c14d5a3c5c6aac186890a8d4819fad78f3e0c41a4e0cd3a2537dd6e91493952fb056fa434be7c!]
     GORELEASER_KEY: ENCRYPTED[!9b80b6ef684ceaf40edd4c7af93014ee156c8aba7e6e5795f41c482729887b5c31f36b651491d790f1f668670888d9fd!]

Package.resolved

@@ -126,6 +126,15 @@
         "version" : "0.9.2"
       }
     },
+    {
+      "identity" : "swiftdate",
+      "kind" : "remoteSourceControl",
+      "location" : "https://github.com/malcommac/SwiftDate",
+      "state" : {
+        "revision" : "6190d0cefff3013e77ed567e6b074f324e5c5bf5",
+        "version" : "6.3.1"
+      }
+    },
     {
       "identity" : "xctest-dynamic-overlay",
       "kind" : "remoteSourceControl",

Package.swift

@@ -15,6 +15,7 @@ let package = Package(
     .package(url: "https://github.com/pointfreeco/swift-parsing", from: "0.9.2"),
     .package(url: "https://github.com/swift-server/async-http-client", from: "1.11.4"),
     .package(url: "https://github.com/apple/swift-algorithms", from: "1.0.0"),
+    .package(url: "https://github.com/malcommac/SwiftDate", from: "6.3.1")
   ],
   targets: [
     .executableTarget(name: "tart", dependencies: [
@@ -23,6 +24,7 @@ let package = Package(
       .product(name: "AsyncHTTPClient", package: "async-http-client"),
       .product(name: "Dynamic", package: "Dynamic"),
       .product(name: "Parsing", package: "swift-parsing"),
+      .product(name: "SwiftDate", package: "SwiftDate"),
     ]),
     .testTarget(name: "TartTests", dependencies: ["tart"])
   ]

Sources/tart/Commands/Clone.swift

@@ -11,6 +11,9 @@ struct Clone: AsyncParsableCommand {
   @Argument(help: "new VM name")
   var newName: String
 
+  @Flag(help: "connect to the OCI registry via insecure HTTP protocol")
+  var insecure: Bool = false
+
   func validate() throws {
     if newName.contains("/") {
       throw ValidationError("<new-name> should be a local name")
@@ -24,7 +27,7 @@ struct Clone: AsyncParsableCommand {
 
       if let remoteName = try? RemoteName(sourceName), !ociStorage.exists(remoteName) {
         // Pull the VM in case it's OCI-based and doesn't exist locally yet
-        let registry = try Registry(host: remoteName.host, namespace: remoteName.namespace)
+        let registry = try Registry(host: remoteName.host, namespace: remoteName.namespace, insecure: insecure)
         try await ociStorage.pull(remoteName, registry: registry)
       }
 

Sources/tart/Commands/List.swift

@@ -10,7 +10,7 @@ struct List: AsyncParsableCommand {
       print("Source\tName")
 
       displayTable("local", try VMStorageLocal().list())
-      displayTable("oci", try VMStorageOCI().list())
+      displayTable("oci", try VMStorageOCI().list().map { (name, vmDir, _) in (name, vmDir) })
 
       Foundation.exit(0)
     } catch {

Sources/tart/Commands/Prune.swift

@@ -0,0 +1,101 @@
+import ArgumentParser
+import Dispatch
+import SwiftUI
+import SwiftDate
+
+struct Prune: AsyncParsableCommand {
+  static var configuration = CommandConfiguration(abstract: "Prune OCI and IPSW caches")
+
+  @Option(help: ArgumentHelp("Remove cache entries last accessed more than n days ago",
+    discussion: "For example, --older-than=7 will remove entries that weren't accessed by Tart in the last 7 days.",
+    valueName: "n"))
+  var olderThan: UInt?
+
+  @Option(help: ArgumentHelp("Remove least recently used cache entries that do not fit the specified cache size budget n, expressed in gigabytes",
+    discussion: "For example, --cache-budget=50 will effectively shrink all caches to a total size of 50 gigabytes.",
+    valueName: "n"))
+  var cacheBudget: UInt?
+
+  func validate() throws {
+    if olderThan == nil && cacheBudget == nil {
+      throw ValidationError("at least one criteria must be specified")
+    }
+  }
+
+  func run() async throws {
+    do {
+      // Clean up cache entries based on last accessed date
+      if let olderThan = olderThan {
+        let olderThanInterval = Int(exactly: olderThan)!.days.timeInterval
+        let olderThanDate = Date().addingTimeInterval(olderThanInterval)
+
+        try Prune.pruneOlderThan(olderThanDate: olderThanDate)
+      }
+
+      // Clean up cache entries based on imposed cache size limit and entry's last accessed date
+      if let cacheBudget = cacheBudget {
+        try Prune.pruneCacheBudget(cacheBudgetBytes: UInt64(cacheBudget) * 1024 * 1024 * 1024)
+      }
+
+      Foundation.exit(0)
+    } catch {
+      print(error)
+
+      Foundation.exit(1)
+    }
+  }
+
+  static func pruneOlderThan(olderThanDate: Date) throws {
+    let prunableStorages: [PrunableStorage] = [VMStorageOCI(), try IPSWCache()]
+    let prunables: [Prunable] = try prunableStorages.flatMap { try $0.prunables() }
+
+    try prunables.filter { try $0.accessDate() <= olderThanDate }.forEach { try $0.delete() }
+  }
+
+  static func pruneCacheBudget(cacheBudgetBytes: UInt64) throws {
+    let prunableStorages: [PrunableStorage] = [VMStorageOCI(), try IPSWCache()]
+    let prunables: [Prunable] = try prunableStorages
+            .flatMap { try $0.prunables() }
+            .sorted { try $0.accessDate() < $1.accessDate() }
+
+    let cacheUsedBytes = try prunables.map { try $0.sizeBytes() }.reduce(0, +)
+    var cacheReclaimedBytes: Int = 0
+
+    var it = prunables.makeIterator()
+
+    while (cacheUsedBytes - cacheReclaimedBytes) > cacheBudgetBytes {
+      guard let prunable = it.next() else {
+        break
+      }
+
+      cacheReclaimedBytes -= try prunable.sizeBytes()
+      try prunable.delete()
+    }
+  }
+
+  static func pruneReclaim(reclaimBytes: UInt64) throws {
+    let prunableStorages: [PrunableStorage] = [VMStorageOCI(), try IPSWCache()]
+    let prunables: [Prunable] = try prunableStorages
+            .flatMap { try $0.prunables() }
+            .sorted { try $0.accessDate() < $1.accessDate() }
+
+    // Does it even make sense to start?
+    let cacheUsedBytes = try prunables.map { try $0.sizeBytes() }.reduce(0, +)
+    if cacheUsedBytes < reclaimBytes {
+      return
+    }
+
+    var cacheReclaimedBytes: Int = 0
+
+    var it = prunables.makeIterator()
+
+    while cacheReclaimedBytes <= reclaimBytes {
+      guard let prunable = it.next() else {
+        break
+      }
+
+      cacheReclaimedBytes -= try prunable.sizeBytes()
+      try prunable.delete()
+    }
+  }
+}

Sources/tart/Commands/Pull.swift

@@ -8,6 +8,9 @@ struct Pull: AsyncParsableCommand {
   @Argument(help: "remote VM name")
   var remoteName: String
 
+  @Flag(help: "connect to the OCI registry via insecure HTTP protocol")
+  var insecure: Bool = false
+
   func run() async throws {
     do {
       // Be more liberal when accepting local image as argument,
@@ -19,7 +22,7 @@ struct Pull: AsyncParsableCommand {
       }
 
       let remoteName = try RemoteName(remoteName)
-      let registry = try Registry(host: remoteName.host, namespace: remoteName.namespace)
+      let registry = try Registry(host: remoteName.host, namespace: remoteName.namespace, insecure: insecure)
 
       defaultLogger.appendNewLine("pulling \(remoteName)...")
 

Sources/tart/IPSWCache.swift

@@ -0,0 +1,20 @@
+import Foundation
+import Virtualization
+
+class IPSWCache: PrunableStorage {
+  let baseURL: URL
+
+  init() throws {
+    baseURL = Config().tartCacheDir.appendingPathComponent("IPSWs", isDirectory: true)
+    try FileManager.default.createDirectory(at: baseURL, withIntermediateDirectories: true)
+  }
+
+  func locationFor(image: VZMacOSRestoreImage) -> URL {
+    baseURL.appendingPathComponent("\(image.buildVersion).ipsw", isDirectory: false)
+  }
+
+  func prunables() throws -> [Prunable] {
+    try FileManager.default.contentsOfDirectory(at: baseURL, includingPropertiesForKeys: nil)
+            .filter { $0.lastPathComponent.hasSuffix(".ipsw")}
+  }
+}

Sources/tart/Prunable.swift

@@ -0,0 +1,11 @@
+import Foundation
+
+protocol PrunableStorage {
+  func prunables() throws -> [Prunable]
+}
+
+protocol Prunable {
+  func delete() throws
+  func accessDate() throws -> Date
+  func sizeBytes() throws -> Int
+}

Sources/tart/Root.swift

@@ -16,6 +16,7 @@ struct Root: AsyncParsableCommand {
       IP.self,
       Pull.self,
       Push.self,
+      Prune.self,
       Delete.self,
     ])
 

Sources/tart/URL+AccessDate.swift

@@ -0,0 +1,25 @@
+import Foundation
+
+extension URL {
+  func accessDate() throws -> Date {
+    let attrs = try resourceValues(forKeys: [.contentAccessDateKey])
+    return attrs.contentAccessDate!
+  }
+
+  func updateAccessDate(_ accessDate: Date = Date()) throws {
+    let attrs = try resourceValues(forKeys: [.contentAccessDateKey])
+    let modificationDate = attrs.contentAccessDate!
+
+    let times = [accessDate.asTimeval(), modificationDate.asTimeval()]
+    let ret = utimes(path, times)
+    if ret != 0 {
+      throw RuntimeError("utimes(2) failed: \(ret.explanation())")
+    }
+  }
+}
+
+extension Date {
+  func asTimeval() -> timeval {
+    timeval(tv_sec: timeIntervalSince1970.toUnit(.second)!, tv_usec: 0)
+  }
+}

Sources/tart/URL+Prunable.swift

@@ -0,0 +1,11 @@
+import Foundation
+
+extension URL: Prunable {
+  func delete() throws {
+    try FileManager.default.removeItem(at: self)
+  }
+
+  func sizeBytes() throws -> Int {
+    try resourceValues(forKeys: [.totalFileAllocatedSizeKey]).totalFileAllocatedSize!
+  }
+}

Sources/tart/VM.swift

@@ -55,14 +55,11 @@ class VM: NSObject, VZVirtualMachineDelegate, ObservableObject {
       }
     }
 
-
-    let ipswCacheFolder = Config().tartCacheDir.appendingPathComponent("IPSWs", isDirectory: true)
-    try FileManager.default.createDirectory(at: ipswCacheFolder, withIntermediateDirectories: true)
-
-    let expectedIPSWLocation = ipswCacheFolder.appendingPathComponent("\(image.buildVersion).ipsw", isDirectory: false)
+    let expectedIPSWLocation = try IPSWCache().locationFor(image: image)
 
     if FileManager.default.fileExists(atPath: expectedIPSWLocation.path) {
       defaultLogger.appendNewLine("Using cached *.ipsw file...")
+      try expectedIPSWLocation.updateAccessDate()
       return expectedIPSWLocation
     }
 

Sources/tart/VMDirectory+OCI.swift

@@ -117,13 +117,15 @@ extension VMDirectory {
 
     // Read VM's compressed disk as chunks
     // and sequentially upload them as blobs
-    let disk = try FileHandle(forReadingFrom: diskURL)
-    var diskReadBytes: UInt64 = 0
-    let compressingFilter = try InputFilter<Data>(.compress, using: .lz4, bufferCapacity: Self.bufferSizeBytes) { _ in
-      let data = try disk.read(upToCount: Self.bufferSizeBytes)
-      diskReadBytes += UInt64(data?.count ?? 0)
+    let mappedDisk = try Data(contentsOf: diskURL, options: [.alwaysMapped])
+    let mappedDiskSize = mappedDisk.count
+    var mappedDiskReadOffset = 0
+    let compressingFilter = try InputFilter(.compress, using: .lz4, bufferCapacity: Self.bufferSizeBytes) { (length: Int) -> Data? in
+      let bytesRead = min(length, mappedDiskSize - mappedDiskReadOffset)
+      let data = mappedDisk.subdata(in: mappedDiskReadOffset ..< mappedDiskReadOffset + bytesRead)
+      mappedDiskReadOffset += bytesRead
 
-      progress.completedUnitCount += Int64(data?.count ?? 0)
+      progress.completedUnitCount = Int64(mappedDiskReadOffset)
 
       return data
     }
@@ -145,7 +147,7 @@ extension VMDirectory {
     let manifest = OCIManifest(
             config: OCIManifestConfig(size: ociConfigJSON.count, digest: ociConfigDigest),
             layers: layers,
-            uncompressedDiskSize: diskReadBytes
+            uncompressedDiskSize: UInt64(mappedDiskReadOffset)
     )
 
     // Manifest

Sources/tart/VMDirectory.swift

@@ -7,7 +7,7 @@ struct UninitializedVMDirectoryError: Error {
 struct AlreadyInitializedVMDirectoryError: Error {
 }
 
-struct VMDirectory {
+struct VMDirectory: Prunable {
   var baseURL: URL
 
   var configURL: URL {
@@ -77,4 +77,16 @@ struct VMDirectory {
     try diskFileHandle.truncate(atOffset: UInt64(sizeGB) * 1000 * 1000 * 1000)
     try diskFileHandle.close()
   }
+
+  func delete() throws {
+    try FileManager.default.removeItem(at: baseURL)
+  }
+
+  func accessDate() throws -> Date {
+    try baseURL.accessDate()
+  }
+
+  func sizeBytes() throws -> Int {
+    try configURL.sizeBytes() + diskURL.sizeBytes() + nvramURL.sizeBytes()
+  }
 }

Sources/tart/VMStorageOCI.swift

@@ -1,6 +1,6 @@
 import Foundation
 
-class VMStorageOCI {
+class VMStorageOCI: PrunableStorage {
   let baseURL = Config().tartCacheDir.appendingPathComponent("OCIs", isDirectory: true)
 
   private func vmURL(_ name: RemoteName) -> URL {
@@ -16,6 +16,8 @@ class VMStorageOCI {
 
     try vmDir.validate()
 
+    try vmDir.baseURL.updateAccessDate()
+
     return vmDir
   }
 
@@ -42,8 +44,8 @@ class VMStorageOCI {
     try FileManager.default.removeItem(at: vmURL(name))
   }
 
-  func list() throws -> [(String, VMDirectory)] {
-    var result: [(String, VMDirectory)] = Array()
+  func list() throws -> [(String, VMDirectory, Bool)] {
+    var result: [(String, VMDirectory, Bool)] = Array()
 
     guard let enumerator = FileManager.default.enumerator(at: baseURL,
       includingPropertiesForKeys: [.isSymbolicLinkKey], options: [.producesRelativePathURLs]) else {
@@ -60,18 +62,23 @@ class VMStorageOCI {
       let parts = [foundURL.deletingLastPathComponent().relativePath, foundURL.lastPathComponent]
       var name: String
 
-      if try foundURL.resourceValues(forKeys: [.isSymbolicLinkKey]).isSymbolicLink! {
+      let isSymlink = try foundURL.resourceValues(forKeys: [.isSymbolicLinkKey]).isSymbolicLink!
+      if isSymlink {
         name = parts.joined(separator: ":")
       } else {
         name = parts.joined(separator: "@")
       }
 
-      result.append((name, vmDir))
+      result.append((name, vmDir, isSymlink))
     }
 
     return result
   }
 
+  func prunables() throws -> [Prunable] {
+    try list().filter { (_, _, isSymlink) in !isSymlink }.map { (_, vmDir, _) in vmDir }
+  }
+
   func pull(_ name: RemoteName, registry: Registry) async throws {
     defaultLogger.appendNewLine("pulling manifest...")
 
@@ -82,6 +89,19 @@ class VMStorageOCI {
 
     if !exists(digestName) {
       let tmpVMDir = try VMDirectory.temporary()
+
+      // Try to reclaim some cache space if we know the VM size in advance
+      if let uncompressedDiskSize = manifest.uncompressedDiskSize() {
+        let requiredCapacityBytes = UInt64(uncompressedDiskSize + 128 * 1024 * 1024)
+
+        let attrs = try tmpVMDir.baseURL.resourceValues(forKeys: [.volumeAvailableCapacityForImportantUsageKey])
+        let availableCapacityBytes = UInt64(attrs.volumeAvailableCapacityForImportantUsage!)
+
+        if availableCapacityBytes < requiredCapacityBytes {
+          try Prune.pruneReclaim(reclaimBytes: requiredCapacityBytes - availableCapacityBytes)
+        }
+      }
+
       try await withTaskCancellationHandler(operation: {
         try await tmpVMDir.pullFromRegistry(registry: registry, manifest: manifest)
         try move(digestName, from: tmpVMDir)

Tests/TartTests/URLAccessDateTests.swift

@@ -0,0 +1,22 @@
+import XCTest
+@testable import tart
+
+final class URLAccessDateTests: XCTestCase {
+  func testGetAndSetAccessTime() throws {
+    // Create a temporary file
+    let tmpDir = URL(fileURLWithPath: NSTemporaryDirectory(), isDirectory: true)
+    var tmpFile = tmpDir.appendingPathComponent(UUID().uuidString)
+    FileManager.default.createFile(atPath: tmpFile.path, contents: nil)
+
+    // Ensure it's access date is different than our desired access date
+    let arbitraryDate = Date.init(year: 2008, month: 09, day: 28, hour: 23, minute: 15)
+    XCTAssertNotEqual(arbitraryDate, try tmpFile.accessDate())
+
+    // Set our desired access date for a file
+    try tmpFile.updateAccessDate(arbitraryDate)
+
+    // Ensure the access date has changed to our value
+    tmpFile.removeCachedResourceValue(forKey: .contentAccessDateKey)
+    XCTAssertEqual(arbitraryDate, try tmpFile.accessDate())
+  }
+}