Skip to content

Commit 31a2289

Browse files
authored
[Collections] Fix racy refresh (#3327)
Motivation: `testHappyRefresh` still crashes. See: https://ci.swift.org/job/oss-swift-package-linux-ubuntu-16_04/5991/console `refreshCollections` leads to target trie getting updated. `Trie` is not thread-safe. Modifications: Make `Trie` thread-safe. `TrieNode` should be thread-safe already.
1 parent 3ff6f8b commit 31a2289

File tree

2 files changed

+32
-19
lines changed

2 files changed

+32
-19
lines changed

Sources/Basics/ConcurrencyHelpers.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
/*
22
This source file is part of the Swift.org open source project
3+
34
Copyright (c) 2020 Apple Inc. and the Swift project authors
45
Licensed under Apache License v2.0 with Runtime Library Exception
6+
57
See http://swift.org/LICENSE.txt for license information
68
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
79
*/

Sources/PackageCollections/Storage/Trie.swift

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
/*
22
This source file is part of the Swift.org open source project
3-
Copyright (c) 2020 Apple Inc. and the Swift project authors
3+
4+
Copyright (c) 2020-2021 Apple Inc. and the Swift project authors
45
Licensed under Apache License v2.0 with Runtime Library Exception
6+
57
See http://swift.org/LICENSE.txt for license information
68
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
79
*/
@@ -14,6 +16,7 @@ struct Trie<Document: Hashable> {
1416
private typealias Node = TrieNode<Character, Document>
1517

1618
private let root: Node
19+
private let lock = Lock()
1720

1821
init() {
1922
self.root = Node()
@@ -23,17 +26,19 @@ struct Trie<Document: Hashable> {
2326
func insert(word: String, foundIn document: Document) {
2427
guard !word.isEmpty else { return }
2528

26-
var currentNode = self.root
27-
// Check if word already exists otherwise creates the node path
28-
for character in word.lowercased() {
29-
if let child = currentNode.children[character] {
30-
currentNode = child
31-
} else {
32-
currentNode = currentNode.add(value: character)
29+
self.lock.withLock {
30+
var currentNode = self.root
31+
// Check if word already exists otherwise creates the node path
32+
for character in word.lowercased() {
33+
if let child = currentNode.children[character] {
34+
currentNode = child
35+
} else {
36+
currentNode = currentNode.add(value: character)
37+
}
3338
}
34-
}
3539

36-
currentNode.add(document: document)
40+
currentNode.add(document: document)
41+
}
3742
}
3843

3944
/// Removes word occurrences found in the given document.
@@ -57,7 +62,9 @@ struct Trie<Document: Hashable> {
5762
}
5863
}
5964

60-
removeInSubTrie(root: self.root, document: document)
65+
self.lock.withLock {
66+
removeInSubTrie(root: self.root, document: document)
67+
}
6168
}
6269

6370
/// Removes word occurrences found in matching document(s).
@@ -81,7 +88,9 @@ struct Trie<Document: Hashable> {
8188
}
8289
}
8390

84-
removeInSubTrie(root: self.root, where: predicate)
91+
self.lock.withLock {
92+
removeInSubTrie(root: self.root, where: predicate)
93+
}
8594
}
8695

8796
/// Checks if the trie contains the exact word or words with matching prefix.
@@ -149,15 +158,17 @@ struct Trie<Document: Hashable> {
149158
private func findLastNodeOf(word: String) -> Node? {
150159
guard !word.isEmpty else { return nil }
151160

152-
var currentNode = self.root
153-
// Traverse down the trie as far as we can
154-
for character in word.lowercased() {
155-
guard let child = currentNode.children[character] else {
156-
return nil
161+
return self.lock.withLock {
162+
var currentNode = self.root
163+
// Traverse down the trie as far as we can
164+
for character in word.lowercased() {
165+
guard let child = currentNode.children[character] else {
166+
return nil
167+
}
168+
currentNode = child
157169
}
158-
currentNode = child
170+
return currentNode
159171
}
160-
return currentNode
161172
}
162173
}
163174

0 commit comments

Comments
 (0)