Skip to content

Commit 6bcbf11

Browse files
authored
Bring back "Lock scratch directory during tool execution" (#7291)
This reverts commit e92df2d and brings back the changes from #7269.
1 parent f385144 commit 6bcbf11

File tree

2 files changed

+83
-6
lines changed

2 files changed

+83
-6
lines changed

Sources/Basics/FileSystem/FileSystem+Extensions.swift

+13-2
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,8 @@ extension FileSystem {
196196
}
197197

198198
/// Execute the given block while holding the lock.
199-
public func withLock<T>(on path: AbsolutePath, type: FileLock.LockType, _ body: () throws -> T) throws -> T {
200-
try self.withLock(on: path.underlying, type: type, body)
199+
public func withLock<T>(on path: AbsolutePath, type: FileLock.LockType, blocking: Bool = true, _ body: () throws -> T) throws -> T {
200+
try self.withLock(on: path.underlying, type: type, blocking: blocking, body)
201201
}
202202

203203
/// Returns any known item replacement directories for a given path. These may be used by platform-specific
@@ -616,3 +616,14 @@ extension FileSystem {
616616
try self.removeFileTree(tempDirectory)
617617
}
618618
}
619+
620+
// MARK: - Locking
621+
622+
extension FileLock {
623+
public static func prepareLock(
624+
fileToLock: AbsolutePath,
625+
at lockFilesDirectory: AbsolutePath? = nil
626+
) throws -> FileLock {
627+
return try Self.prepareLock(fileToLock: fileToLock.underlying, at: lockFilesDirectory?.underlying)
628+
}
629+
}

Sources/CoreCommands/SwiftTool.swift

+70-4
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,11 @@ import Musl
3939
#endif
4040

4141
import func TSCBasic.exec
42+
import class TSCBasic.FileLock
4243
import protocol TSCBasic.OutputByteStream
4344
import class TSCBasic.Process
4445
import enum TSCBasic.ProcessEnv
46+
import enum TSCBasic.ProcessLockError
4547
import var TSCBasic.stderrStream
4648
import class TSCBasic.TerminalController
4749
import class TSCBasic.ThreadSafeOutputByteStream
@@ -80,6 +82,7 @@ public typealias WorkspaceLoaderProvider = (_ fileSystem: FileSystem, _ observab
8082
-> WorkspaceLoader
8183

8284
public protocol _SwiftCommand {
85+
var globalOptions: GlobalOptions { get }
8386
var toolWorkspaceConfiguration: ToolWorkspaceConfiguration { get }
8487
var workspaceDelegateProvider: WorkspaceDelegateProvider { get }
8588
var workspaceLoaderProvider: WorkspaceLoaderProvider { get }
@@ -93,8 +96,6 @@ extension _SwiftCommand {
9396
}
9497

9598
public protocol SwiftCommand: ParsableCommand, _SwiftCommand {
96-
var globalOptions: GlobalOptions { get }
97-
9899
func run(_ swiftTool: SwiftTool) throws
99100
}
100101

@@ -108,6 +109,10 @@ extension SwiftCommand {
108109
workspaceDelegateProvider: self.workspaceDelegateProvider,
109110
workspaceLoaderProvider: self.workspaceLoaderProvider
110111
)
112+
113+
// We use this to attempt to catch misuse of the locking APIs since we only release the lock from here.
114+
swiftTool.setNeedsLocking()
115+
111116
swiftTool.buildSystemProvider = try buildSystemProvider(swiftTool)
112117
var toolError: Error? = .none
113118
do {
@@ -119,6 +124,8 @@ extension SwiftCommand {
119124
toolError = error
120125
}
121126

127+
swiftTool.releaseLockIfNeeded()
128+
122129
// wait for all observability items to process
123130
swiftTool.waitForObservabilityEvents(timeout: .now() + 5)
124131

@@ -129,21 +136,24 @@ extension SwiftCommand {
129136
}
130137

131138
public protocol AsyncSwiftCommand: AsyncParsableCommand, _SwiftCommand {
132-
var globalOptions: GlobalOptions { get }
133-
134139
func run(_ swiftTool: SwiftTool) async throws
135140
}
136141

137142
extension AsyncSwiftCommand {
138143
public static var _errorLabel: String { "error" }
139144

145+
// FIXME: It doesn't seem great to have this be duplicated with `SwiftCommand`.
140146
public func run() async throws {
141147
let swiftTool = try SwiftTool(
142148
options: globalOptions,
143149
toolWorkspaceConfiguration: self.toolWorkspaceConfiguration,
144150
workspaceDelegateProvider: self.workspaceDelegateProvider,
145151
workspaceLoaderProvider: self.workspaceLoaderProvider
146152
)
153+
154+
// We use this to attempt to catch misuse of the locking APIs since we only release the lock from here.
155+
swiftTool.setNeedsLocking()
156+
147157
swiftTool.buildSystemProvider = try buildSystemProvider(swiftTool)
148158
var toolError: Error? = .none
149159
do {
@@ -155,6 +165,8 @@ extension AsyncSwiftCommand {
155165
toolError = error
156166
}
157167

168+
swiftTool.releaseLockIfNeeded()
169+
158170
// wait for all observability items to process
159171
swiftTool.waitForObservabilityEvents(timeout: .now() + 5)
160172

@@ -396,6 +408,9 @@ public final class SwiftTool {
396408
return workspace
397409
}
398410

411+
// Before creating the workspace, we need to acquire a lock on the build directory.
412+
try self.acquireLockIfNeeded()
413+
399414
if options.resolver.skipDependencyUpdate {
400415
self.observabilityScope.emit(warning: "'--skip-update' option is deprecated and will be removed in a future release")
401416
}
@@ -866,6 +881,57 @@ public final class SwiftTool {
866881
case success
867882
case failure
868883
}
884+
885+
// MARK: - Locking
886+
887+
// This is used to attempt to prevent accidental misuse of the locking APIs.
888+
private enum WorkspaceLockState {
889+
case unspecified
890+
case needsLocking
891+
case locked
892+
case unlocked
893+
}
894+
895+
private var workspaceLockState: WorkspaceLockState = .unspecified
896+
private var workspaceLock: FileLock?
897+
898+
fileprivate func setNeedsLocking() {
899+
assert(workspaceLockState == .unspecified, "attempting to `setNeedsLocking()` from unexpected state: \(workspaceLockState)")
900+
workspaceLockState = .needsLocking
901+
}
902+
903+
fileprivate func acquireLockIfNeeded() throws {
904+
assert(workspaceLockState == .needsLocking, "attempting to `acquireLockIfNeeded()` from unexpected state: \(workspaceLockState)")
905+
guard workspaceLock == nil else {
906+
throw InternalError("acquireLockIfNeeded() called multiple times")
907+
}
908+
workspaceLockState = .locked
909+
910+
let workspaceLock = try FileLock.prepareLock(fileToLock: self.scratchDirectory)
911+
912+
// Try a non-blocking lock first so that we can inform the user about an already running SwiftPM.
913+
do {
914+
try workspaceLock.lock(type: .exclusive, blocking: false)
915+
} catch let ProcessLockError.unableToAquireLock(errno) {
916+
if errno == EWOULDBLOCK {
917+
self.outputStream.write("Another instance of SwiftPM is already running using '\(self.scratchDirectory)', waiting until that process has finished execution...".utf8)
918+
self.outputStream.flush()
919+
920+
// Only if we fail because there's an existing lock we need to acquire again as blocking.
921+
try workspaceLock.lock(type: .exclusive, blocking: true)
922+
}
923+
}
924+
925+
self.workspaceLock = workspaceLock
926+
}
927+
928+
fileprivate func releaseLockIfNeeded() {
929+
// Never having acquired the lock is not an error case.
930+
assert(workspaceLockState == .locked || workspaceLockState == .needsLocking, "attempting to `releaseLockIfNeeded()` from unexpected state: \(workspaceLockState)")
931+
workspaceLockState = .unlocked
932+
933+
workspaceLock?.unlock()
934+
}
869935
}
870936

871937
/// Returns path of the nearest directory containing the manifest file w.r.t

0 commit comments

Comments
 (0)