Skip to content

ProcessID enhacement #8618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions Sources/Basics/Concurrency/PID.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 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
//
//===----------------------------------------------------------------------===//

import Foundation

public protocol PIDFileHandler {
var scratchDirectory: AbsolutePath { get set }

init(scratchDirectory: AbsolutePath)

func readPID() -> Int32?
func deletePIDFile() throws
func writePID(pid: Int32) throws
func getCurrentPID() -> Int32
}

public struct PIDFile: PIDFileHandler {
public var scratchDirectory: AbsolutePath

public init(scratchDirectory: AbsolutePath) {
self.scratchDirectory = scratchDirectory
}

/// Return the path of the PackageManager.lock.pid file where the PID is located
private var lockFilePath: AbsolutePath {
self.scratchDirectory.appending(component: "PackageManager.lock.pid")
}

/// Read the pid file
public func readPID() -> Int32? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): consider adding small tests that would validate all code paths in isolation.

// Check if the file exists
let filePath = self.lockFilePath.pathString
guard FileManager.default.fileExists(atPath: filePath) else {
print("File does not exist at path: \(filePath)")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (blocking): I think that these print statements need to be emitted through observability scopes instead of printing directly to stdout here.

return nil
}

do {
// Read the contents of the file
let pidString = try String(contentsOf: lockFilePath.asURL, encoding: .utf8)
.trimmingCharacters(in: .whitespacesAndNewlines)

// Check if the PID string can be converted to an Int32
if let pid = Int32(pidString) {
return pid
} else {
return nil
}
} catch {
// Catch any errors and print them
return nil
}
}

/// Get the current PID of the proces
public func getCurrentPID() -> Int32 {
return ProcessInfo.processInfo.processIdentifier
}

/// Write .pid file containing PID of process currently using .build directory
public func writePID(pid: Int32) throws {
let parent = self.lockFilePath.parentDirectory
try FileManager.default.createDirectory(
at: parent.asURL,
withIntermediateDirectories: true,
attributes: nil
)

try "\(pid)".write(to: self.lockFilePath.asURL, atomically: true, encoding: .utf8)
}

/// Delete PID file at URL
public func deletePIDFile() throws {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): consider adding small (ie: unit) tests that validate this function in isolation:

  • self.lockFilePath exists before calling function
  • self.locFilePath does no exists before calling function

try FileManager.default.removeItem(at: self.lockFilePath.asURL)
}
}
34 changes: 26 additions & 8 deletions Sources/CoreCommands/SwiftCommandState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,8 @@ public final class SwiftCommandState {

private let hostTriple: Basics.Triple?

private let pidManipulator: PIDFileHandler

package var preferredBuildConfiguration = BuildConfiguration.debug

/// Create an instance of this tool.
Expand Down Expand Up @@ -324,7 +326,8 @@ public final class SwiftCommandState {
createPackagePath: Bool,
hostTriple: Basics.Triple? = nil,
fileSystem: any FileSystem = localFileSystem,
environment: Environment = .current
environment: Environment = .current,
pidManipulator: PIDFileHandler? = nil
) throws {
self.hostTriple = hostTriple
self.fileSystem = fileSystem
Expand Down Expand Up @@ -408,6 +411,8 @@ public final class SwiftCommandState {
explicitDirectory: options.locations.swiftSDKsDirectory ?? options.locations.deprecatedSwiftSDKsDirectory
)

self.pidManipulator = pidManipulator ?? PIDFile(scratchDirectory: self.scratchDirectory)

// set global process logging handler
AsyncProcess.loggingHandler = { self.observabilityScope.emit(debug: $0) }
}
Expand Down Expand Up @@ -1059,16 +1064,16 @@ public final class SwiftCommandState {
let workspaceLock = try FileLock.prepareLock(fileToLock: self.scratchDirectory)
let lockFile = self.scratchDirectory.appending(".lock").pathString

var lockAcquired = false

// Try a non-blocking lock first so that we can inform the user about an already running SwiftPM.
do {
try workspaceLock.lock(type: .exclusive, blocking: false)
let pid = ProcessInfo.processInfo.processIdentifier
try? String(pid).write(toFile: lockFile, atomically: true, encoding: .utf8)
lockAcquired = true
} catch ProcessLockError.unableToAquireLock(let errno) {
if errno == EWOULDBLOCK {
let lockingPID = try? String(contentsOfFile: lockFile, encoding: .utf8)
let pidInfo = lockingPID.map { "(PID: \($0)) " } ?? ""

let existingProcessPID = self.pidManipulator.readPID()
let pidInfo = existingProcessPID.map { "(PID: \($0)) " } ?? ""
if self.options.locations.ignoreLock {
self.outputStream
.write(
Expand All @@ -1087,13 +1092,20 @@ public final class SwiftCommandState {
// Only if we fail because there's an existing lock we need to acquire again as blocking.
try workspaceLock.lock(type: .exclusive, blocking: true)

let pid = ProcessInfo.processInfo.processIdentifier
try? String(pid).write(toFile: lockFile, atomically: true, encoding: .utf8)
lockAcquired = true
}
}
}

self.workspaceLock = workspaceLock

if lockAcquired || self.options.locations.ignoreLock {
do {
try self.pidManipulator.writePID(pid: self.pidManipulator.getCurrentPID())
} catch {
self.observabilityScope.emit(warning: "Failed to write to PID file: \(error)")
}
}
}

fileprivate func releaseLockIfNeeded() {
Expand All @@ -1105,6 +1117,12 @@ public final class SwiftCommandState {
self.workspaceLockState = .unlocked

self.workspaceLock?.unlock()

do {
try self.pidManipulator.deletePIDFile()
} catch {
self.observabilityScope.emit(warning: "Failed to delete PID file: \(error)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: this message may be misleading. As a developer, I might wonder "what is the PID file" . What is the side effect of the existence of this file? Will the next "write" fail? if not, can we omit notifying the user?

}
}
}

Expand Down
39 changes: 39 additions & 0 deletions Tests/CommandsTests/SwiftCommandStateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,45 @@ final class SwiftCommandStateTests: CommandsTestCase {
}
}
}

func testPIDFileHandlerLifecycle() async throws {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: These new tests are validating the PIDFile structure. Can we move the tests to a new location? Maybe under Tests/Basics/Concurrency/PIDTests.swift

Also, consider using Swift Testing instead of XCTest

try withTemporaryDirectory { tmpDir in
let scratchPath = tmpDir.appending(component: "scratch")
try localFileSystem.createDirectory(scratchPath)

let pidHandler = PIDFile(scratchDirectory: scratchPath)

// Ensure no PID file exists initially
XCTAssertNil(pidHandler.readPID(), "No PID should exist initially")

// Write current PID
let currentPID = pidHandler.getCurrentPID()
try pidHandler.writePID(pid: currentPID)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: can we add a test where multiple writePID(...) are calls sequentially?


// Read PID back
let readPID = pidHandler.readPID()
XCTAssertEqual(readPID, currentPID, "PID read should match written PID")

// Delete the file
try pidHandler.deletePIDFile()

// Ensure file is gone
XCTAssertNil(pidHandler.readPID(), "PID should be nil after deletion")
}
}

func testMalformedPIDFile() async throws {
try withTemporaryDirectory { tmpDir in
let scratchPath = tmpDir.appending(component: "scratch")
try localFileSystem.createDirectory(scratchPath)

let pidPath = scratchPath.appending(component: "PackageManager.lock.pid")
try localFileSystem.writeFileContents(pidPath, bytes: "notanumber")

let pidHandler = PIDFile(scratchDirectory: scratchPath)
XCTAssertNil(pidHandler.readPID(), "Malformed PID file should result in nil")
}
}
}

extension SwiftCommandState {
Expand Down