From 038c0e109f7824bb31700fdfe1365ccdcda4ebec Mon Sep 17 00:00:00 2001 From: Xerol Wong Date: Fri, 6 Mar 2026 20:23:59 +0900 Subject: [PATCH 1/2] fix: prevent race condition when installing multiple Xcode versions concurrently Each XIP is now extracted into a version-specific directory (e.g. Xcode-16.0-extract/) instead of sharing the same parent directory, eliminating the race where one version's extracted Xcode.app could be moved to another version's destination path. Co-Authored-By: Claude Sonnet 4.6 --- Xcodes/Backend/AppState+Install.swift | 29 +++++++++++++++++++-------- Xcodes/Backend/Environment.swift | 6 +++--- XcodesTests/AppStateTests.swift | 2 +- XcodesTests/Environment+Mock.swift | 2 +- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/Xcodes/Backend/AppState+Install.swift b/Xcodes/Backend/AppState+Install.swift index 5e2a0742..bda98556 100644 --- a/Xcodes/Backend/AppState+Install.swift +++ b/Xcodes/Backend/AppState+Install.swift @@ -250,8 +250,17 @@ extension AppState { func unarchiveAndMoveXIP(availableXcode: AvailableXcode, at source: URL, to destination: URL) -> AnyPublisher { self.setInstallationStep(of: availableXcode.version, to: .unarchiving) - - return unxipOrUnxipExperiment(source) + + // Use a version-specific extraction directory to prevent conflicts when + // multiple Xcode versions are installed concurrently. Without this, all + // XIP files extract to the same parent directory as "Xcode.app", causing + // a race condition where one version’s extracted app gets renamed to + // another version’s destination path. + let extractionDirectory = source.deletingLastPathComponent() + .appendingPathComponent("Xcode-\(availableXcode.version)-extract") + try? Current.files.createDirectory(at: extractionDirectory, withIntermediateDirectories: true, attributes: nil) + + return unxipOrUnxipExperiment(source, extractionDirectory: extractionDirectory) .catch { error -> AnyPublisher in if let executionError = error as? ProcessExecutionError { if executionError.standardError.contains("damaged and can’t be expanded") { @@ -269,8 +278,8 @@ extension AppState { .tryMap { output -> URL in self.setInstallationStep(of: availableXcode.version, to: .moving(destination: destination.path)) - let xcodeURL = source.deletingLastPathComponent().appendingPathComponent("Xcode.app") - let xcodeBetaURL = source.deletingLastPathComponent().appendingPathComponent("Xcode-beta.app") + let xcodeURL = extractionDirectory.appendingPathComponent("Xcode.app") + let xcodeBetaURL = extractionDirectory.appendingPathComponent("Xcode-beta.app") if Current.files.fileExists(atPath: xcodeURL.path) { try Current.files.moveItem(at: xcodeURL, to: destination) } @@ -278,6 +287,7 @@ extension AppState { try Current.files.moveItem(at: xcodeBetaURL, to: destination) } + try? Current.files.removeItem(at: extractionDirectory) return destination } .handleEvents(receiveCancel: { @@ -287,17 +297,20 @@ extension AppState { if Current.files.fileExists(atPath: destination.path) { try? Current.files.removeItem(destination) } + if Current.files.fileExists(atPath: extractionDirectory.path) { + try? Current.files.removeItem(extractionDirectory) + } }) .eraseToAnyPublisher() } - - func unxipOrUnxipExperiment(_ source: URL) -> AnyPublisher { + + func unxipOrUnxipExperiment(_ source: URL, extractionDirectory: URL) -> AnyPublisher { if unxipExperiment { // All hard work done by https://github.com/saagarjha/unxip // Compiled to binary with `swiftc -parse-as-library -O unxip.swift` - return Current.shell.unxipExperiment(source) + return Current.shell.unxipExperiment(source, extractionDirectory) } else { - return Current.shell.unxip(source) + return Current.shell.unxip(source, extractionDirectory) } } diff --git a/Xcodes/Backend/Environment.swift b/Xcodes/Backend/Environment.swift index b515e11e..251575cf 100644 --- a/Xcodes/Backend/Environment.swift +++ b/Xcodes/Backend/Environment.swift @@ -25,7 +25,7 @@ public struct Environment { public var Current = Environment() public struct Shell { - public var unxip: (URL) -> AnyPublisher = { Process.run(Path.root.usr.bin.xip, workingDirectory: $0.deletingLastPathComponent(), "--expand", "\($0.path)") } + public var unxip: (URL, URL) -> AnyPublisher = { xipURL, workingDirectory in Process.run(Path.root.usr.bin.xip, workingDirectory: workingDirectory, "--expand", "\(xipURL.path)") } public var spctlAssess: (URL) -> AnyPublisher = { Process.run(Path.root.usr.sbin.spctl, "--assess", "--verbose", "--type", "execute", "\($0.path)") } public var codesignVerify: (URL) -> AnyPublisher = { Process.run(Path.root.usr.bin.codesign, "-vv", "-d", "\($0.path)") } public var buildVersion: () -> AnyPublisher = { Process.run(Path.root.usr.bin.sw_vers, "-buildVersion") } @@ -191,9 +191,9 @@ public struct Shell { } - public var unxipExperiment: (URL) -> AnyPublisher = { url in + public var unxipExperiment: (URL, URL) -> AnyPublisher = { xipURL, workingDirectory in let unxipPath = Path(url: Bundle.main.url(forAuxiliaryExecutable: "unxip")!)! - return Process.run(unxipPath.url, workingDirectory: url.deletingLastPathComponent(), ["\(url.path)"]) + return Process.run(unxipPath.url, workingDirectory: workingDirectory, ["\(xipURL.path)"]) } public var downloadRuntime: (String, String, String?) -> AsyncThrowingStream = { platform, version, architecture in diff --git a/XcodesTests/AppStateTests.swift b/XcodesTests/AppStateTests.swift index 4be9ca32..9421e584 100644 --- a/XcodesTests/AppStateTests.swift +++ b/XcodesTests/AppStateTests.swift @@ -291,7 +291,7 @@ class AppStateTests: XCTestCase { } func test_Install_NotEnoughFreeSpace() throws { - Current.shell.unxip = { _ in + Current.shell.unxip = { _, _ in Fail(error: ProcessExecutionError( process: Process(), standardOutput: "xip: signing certificate was \"Development Update\" (validation not attempted)", diff --git a/XcodesTests/Environment+Mock.swift b/XcodesTests/Environment+Mock.swift index f030d796..8846e469 100644 --- a/XcodesTests/Environment+Mock.swift +++ b/XcodesTests/Environment+Mock.swift @@ -18,7 +18,7 @@ extension Shell { static var processOutputMock: ProcessOutput = (0, "", "") static var mock = Shell( - unxip: { _ in return Just(Shell.processOutputMock).setFailureType(to: Error.self).eraseToAnyPublisher() }, + unxip: { _, _ in return Just(Shell.processOutputMock).setFailureType(to: Error.self).eraseToAnyPublisher() }, spctlAssess: { _ in return Just(Shell.processOutputMock).setFailureType(to: Error.self).eraseToAnyPublisher() }, codesignVerify: { _ in return Just(Shell.processOutputMock).setFailureType(to: Error.self).eraseToAnyPublisher() }, buildVersion: { return Just(Shell.processOutputMock).setFailureType(to: Error.self).eraseToAnyPublisher() }, From 7b6e570c33c7b3978f79861940f77fc0bc5aa063 Mon Sep 17 00:00:00 2001 From: Xerol Wong Date: Sat, 7 Mar 2026 18:07:21 +0900 Subject: [PATCH 2/2] test: add regression for concurrent XIP extraction race --- XcodesTests/AppStateTests.swift | 131 ++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/XcodesTests/AppStateTests.swift b/XcodesTests/AppStateTests.swift index 9421e584..7258dd40 100644 --- a/XcodesTests/AppStateTests.swift +++ b/XcodesTests/AppStateTests.swift @@ -325,4 +325,135 @@ class AppStateTests: XCTestCase { XCTFail() } } + + func test_UnarchiveAndMoveXIP_ConcurrentInstalls_DoNotShareExtractionDirectory() throws { + enum CollisionError: Error { + case extractionDirectoryCollision(String) + } + + let stateQueue = DispatchQueue(label: "AppStateTests.ConcurrentInstallState") + var existingPaths = Set() + var activeExtractionDirectories = Set() + var usedExtractionDirectories: [String] = [] + var failures: [Error] = [] + var movedDestinations: [String] = [] + var cancellables = Set() + + Current.files.createDirectory = { directoryURL, _, _ in + stateQueue.sync { + existingPaths.insert(directoryURL.path) + } + } + Current.files.fileExistsAtPath = { path in + stateQueue.sync { + existingPaths.contains(path) + } + } + Current.files.moveItem = { source, destination in + try stateQueue.sync { + guard existingPaths.remove(source.path) != nil else { + throw CocoaError(.fileNoSuchFile) + } + existingPaths.insert(destination.path) + } + } + Current.files.removeItem = { url in + stateQueue.sync { + existingPaths.remove(url.path) + existingPaths = Set(existingPaths.filter { !$0.hasPrefix(url.path + "/") }) + } + } + + Current.shell.unxip = { _, extractionDirectory in + Deferred { + Future { promise in + let hasCollision = stateQueue.sync { () -> Bool in + usedExtractionDirectories.append(extractionDirectory.path) + if activeExtractionDirectories.contains(extractionDirectory.path) { + return true + } + activeExtractionDirectories.insert(extractionDirectory.path) + return false + } + + if hasCollision { + promise(.failure(CollisionError.extractionDirectoryCollision(extractionDirectory.path))) + return + } + + DispatchQueue.global().asyncAfter(deadline: .now() + 0.05) { + stateQueue.sync { + existingPaths.insert(extractionDirectory.appendingPathComponent("Xcode.app").path) + activeExtractionDirectories.remove(extractionDirectory.path) + } + promise(.success((0, "", ""))) + } + } + } + .eraseToAnyPublisher() + } + + let sourceDirectory = URL(fileURLWithPath: "/tmp/xcodes-tests", isDirectory: true) + let availableXcode16_0 = AvailableXcode( + version: Version("16.0.0")!, + url: URL(string: "https://developer.apple.com/download/Xcode-16.0.xip")!, + filename: "Xcode-16.0.xip", + releaseDate: nil + ) + let availableXcode16_1 = AvailableXcode( + version: Version("16.1.0")!, + url: URL(string: "https://developer.apple.com/download/Xcode-16.1.xip")!, + filename: "Xcode-16.1.xip", + releaseDate: nil + ) + + let firstDestination = URL(fileURLWithPath: "/Applications/Xcode-16.0.app") + let secondDestination = URL(fileURLWithPath: "/Applications/Xcode-16.1.app") + + let finished = expectation(description: "Both unarchive operations finished") + finished.expectedFulfillmentCount = 2 + + func subscribe(_ publisher: AnyPublisher) { + publisher + .sink(receiveCompletion: { completion in + if case let .failure(error) = completion { + stateQueue.sync { + failures.append(error) + } + } + finished.fulfill() + }, receiveValue: { movedURL in + stateQueue.sync { + movedDestinations.append(movedURL.path) + } + }) + .store(in: &cancellables) + } + + subscribe( + subject.unarchiveAndMoveXIP( + availableXcode: availableXcode16_0, + at: sourceDirectory.appendingPathComponent("Xcode-16.0.xip"), + to: firstDestination + ) + ) + subscribe( + subject.unarchiveAndMoveXIP( + availableXcode: availableXcode16_1, + at: sourceDirectory.appendingPathComponent("Xcode-16.1.xip"), + to: secondDestination + ) + ) + + wait(for: [finished], timeout: 2.0) + + XCTAssertTrue(failures.isEmpty, "Expected no extraction directory collisions, but got \(failures)") + XCTAssertEqual(Set(movedDestinations), Set([firstDestination.path, secondDestination.path])) + + let expectedExtractionDirectories = Set([ + sourceDirectory.appendingPathComponent("Xcode-\(availableXcode16_0.version)-extract").path, + sourceDirectory.appendingPathComponent("Xcode-\(availableXcode16_1.version)-extract").path + ]) + XCTAssertEqual(Set(usedExtractionDirectories), expectedExtractionDirectories) + } }