Skip to content

Commit b23eb6d

Browse files
authored
fix potential race in PackageCollections.refreshCollections (#3233)
motivation: CI crashes in PackageCollectsTest::testBrokenRefresh suggest a race condition in the code changes: * change ThreadSafeArrayStore::apend to return the current count in a thread safe manner * update the callsite to use the count returned from append instead of after the append -- which could be racy rdar://73884751
1 parent dc41d7f commit b23eb6d

File tree

2 files changed

+5
-3
lines changed

2 files changed

+5
-3
lines changed

Sources/Basics/ConcurrencyHelpers.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,11 @@ public final class ThreadSafeArrayStore<Value> {
9595
}
9696
}
9797

98-
public func append(_ item: Value) {
98+
@discardableResult
99+
public func append(_ item: Value) -> Int {
99100
self.lock.withLock {
100101
self.underlying.append(item)
102+
return self.underlying.count
101103
}
102104
}
103105

Sources/PackageCollections/PackageCollections.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,8 @@ public struct PackageCollections: PackageCollectionsProtocol {
104104
let refreshResults = ThreadSafeArrayStore<Result<Model.Collection, Error>>()
105105
sources.forEach { source in
106106
self.refreshCollectionFromSource(source: source) { refreshResult in
107-
refreshResults.append(refreshResult)
108-
if refreshResults.count == sources.count {
107+
let count = refreshResults.append(refreshResult)
108+
if count == sources.count {
109109
let errors = refreshResults.compactMap { $0.failure }
110110
callback(errors.isEmpty ? .success(sources) : .failure(MultipleErrors(errors)))
111111
}

0 commit comments

Comments
 (0)