Skip to content

Commit bb50625

Browse files
committed
use serial queue to order repository lookups
motivation: lookup callback could race and lead to broken resolution. the actual lookup code assume no-cocurrent access but get called on a concurrent queue which could lead to races changes: * change the repository lookup callback queue to a serial one rdar://118239206
1 parent 5b445bc commit bb50625

File tree

2 files changed

+6
-3
lines changed

2 files changed

+6
-3
lines changed

Sources/SourceControl/RepositoryManager.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ public class RepositoryManager: Cancellable {
4040
private let concurrencySemaphore: DispatchSemaphore
4141
/// OperationQueue to park pending lookups
4242
private let lookupQueue: OperationQueue
43+
/// Serial queue to manage order of lookup callbacks
44+
private let serialLookupCallbackQueue = DispatchQueue(label: "org.swift.swiftpm.repository-manager.serial-callback")
4345

4446
/// The filesystem to operate on.
4547
private let fileSystem: FileSystem
@@ -87,7 +89,7 @@ public class RepositoryManager: Cancellable {
8789
// this queue and semaphore is used to limit the amount of concurrent git operations taking place
8890
let maxConcurrentOperations = max(1, maxConcurrentOperations ?? 3*Concurrency.maxOperations/4)
8991
self.lookupQueue = OperationQueue()
90-
self.lookupQueue.name = "org.swift.swiftpm.repository-manager"
92+
self.lookupQueue.name = "org.swift.swiftpm.repository-manager.concurrent"
9193
self.lookupQueue.maxConcurrentOperationCount = maxConcurrentOperations
9294
self.concurrencySemaphore = DispatchSemaphore(value: maxConcurrentOperations)
9395
}
@@ -148,7 +150,8 @@ public class RepositoryManager: Cancellable {
148150
if let pendingLookup = self.pendingLookups[repository] {
149151
self.pendingLookupsLock.unlock()
150152
// chain onto the pending lookup
151-
return pendingLookup.notify(queue: .sharedConcurrent) {
153+
// not the notification is done on a serial queue as the internal lookup assumes no concurrent access
154+
return pendingLookup.notify(queue: self.serialLookupCallbackQueue) {
152155
// at this point the previous lookup should be complete and we can re-lookup
153156
completion(.init(catching: {
154157
try self.lookup(

Tests/BuildTests/BuildPlanTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ final class BuildPlanTests: XCTestCase {
529529
func testPackageNameFlag() throws {
530530
let isFlagSupportedInDriver = try DriverSupport.checkToolchainDriverFlags(flags: ["package-name"], toolchain: UserToolchain.default, fileSystem: localFileSystem)
531531
try fixture(name: "Miscellaneous/PackageNameFlag") { fixturePath in
532-
let (stdout, _) = try executeSwiftBuild(fixturePath.appending("appPkg"), extraArgs: ["-v"])
532+
let (stdout, _) = try executeSwiftBuild(fixturePath.appending("appPkg"), extraArgs: ["--vv"])
533533
XCTAssertMatch(stdout, .contains("-module-name Foo"))
534534
XCTAssertMatch(stdout, .contains("-module-name Zoo"))
535535
XCTAssertMatch(stdout, .contains("-module-name Bar"))

0 commit comments

Comments
 (0)