Skip to content

Commit 6470d3d

Browse files
committed
[Runtime] Fix StableAddressConcurrentReadableHashMap threading crash.
StableAddressConcurrentReadableHashMap::getOrInsert had a race condition in the first lookup, where the snapshot was destroyed before the pointer was extracted from the returned wrapper. Fix this by creating the snapshot outside the if so that it stays alive. rdar://problem/71932487
1 parent 5a584c7 commit 6470d3d

File tree

2 files changed

+65
-3
lines changed

2 files changed

+65
-3
lines changed

include/swift/Runtime/Concurrent.h

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,9 +1146,15 @@ struct StableAddressConcurrentReadableHashMap
11461146
return {lastFound, false};
11471147

11481148
// Optimize for the case where the value already exists.
1149-
if (auto wrapper = this->snapshot().find(key)) {
1150-
LastFound.store(wrapper->Ptr, std::memory_order_relaxed);
1151-
return {wrapper->Ptr, false};
1149+
{
1150+
// Tightly scope the snapshot so it's gone before we call getOrInsert
1151+
// below, otherwise that call will always see an outstanding snapshot and
1152+
// never be able to collect garbage.
1153+
auto snapshot = this->snapshot();
1154+
if (auto wrapper = snapshot.find(key)) {
1155+
LastFound.store(wrapper->Ptr, std::memory_order_relaxed);
1156+
return {wrapper->Ptr, false};
1157+
}
11521158
}
11531159

11541160
// No such element. Insert if needed. Note: another thread may have inserted
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// RUN: %target-run-simple-swift
2+
// REQUIRES: executable_test
3+
4+
// Exercise the metadata cache from multiple threads to shake out any
5+
// concurrency bugs.
6+
7+
import StdlibUnittest
8+
9+
import SwiftPrivateThreadExtras
10+
11+
struct One {}
12+
struct Two {}
13+
14+
struct Cat<T, U> {}
15+
16+
protocol Growable {}
17+
extension Growable {
18+
func grow() -> (Growable, Growable) {
19+
return (Cat<Self, One>(), Cat<Self, Two>())
20+
}
21+
}
22+
23+
extension One: Growable {}
24+
extension Two: Growable {}
25+
extension Cat: Growable {}
26+
27+
var ConcurrentMetadataTestSuite = TestSuite("ConcurrentMetadata")
28+
29+
ConcurrentMetadataTestSuite.test("ConcurrentMetadata") {
30+
let threadCount = 16
31+
let iterationCount = 10000
32+
33+
func threadFunc() {
34+
var array: [Growable] = [One(), Two()]
35+
for i in 0..<iterationCount {
36+
// Each call to grow() creates a new generic metadata specialization which
37+
// will race with creating that same metadata on the other threads.
38+
let (a, b) = array[i].grow()
39+
array.append(a)
40+
array.append(b)
41+
}
42+
}
43+
44+
let threadIDs = (0..<16).map { _ -> ThreadHandle in
45+
let (ret, threadID) = _stdlib_thread_create_block(threadFunc, ())
46+
expectEqual(0, ret)
47+
return threadID!
48+
}
49+
50+
for threadID in threadIDs {
51+
let (ret, _) = _stdlib_thread_join(threadID, Void.self)
52+
expectEqual(0, ret)
53+
}
54+
}
55+
56+
runAllTests()

0 commit comments

Comments
 (0)