Skip to content

Commit 430a9f3

Browse files
committed
[stdlib] _BridgingHashBuffer: Fix memory management
- Have the hash buffer include a reference to the original hash storage instance, along with a copy of its _HashTable, so that its lifetime can be independent from the deferred bridging object. - Convert _BridgingHashBuffer to a ManagedBuffer so that we can easily put reference-counted properties in its header.
1 parent b2d4e80 commit 430a9f3

File tree

3 files changed

+48
-82
lines changed

3 files changed

+48
-82
lines changed

stdlib/public/core/Dictionary.swift

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2831,7 +2831,7 @@ final internal class _SwiftDeferredNSDictionary<Key: Hashable, Value>
28312831
@nonobjc
28322832
private var _bridgedKeys_DoNotUse: AnyObject?
28332833

2834-
// This stored property must be stored at offset zero. We perform atomic
2834+
// This stored property must be stored at offset one. We perform atomic
28352835
// operations on it.
28362836
//
28372837
// Do not access this property directly.
@@ -2858,11 +2858,6 @@ final internal class _SwiftDeferredNSDictionary<Key: Hashable, Value>
28582858
_sanityCheckFailure("don't call this designated initializer")
28592859
}
28602860

2861-
deinit {
2862-
_bridgedKeys?.deinitialize(elementsFrom: native.hashTable)
2863-
_bridgedValues?.deinitialize(elementsFrom: native.hashTable)
2864-
}
2865-
28662861
@nonobjc
28672862
private var _bridgedKeysPtr: UnsafeMutablePointer<AnyObject?> {
28682863
return _getUnsafePointerToStoredProperties(self)
@@ -2894,20 +2889,14 @@ final internal class _SwiftDeferredNSDictionary<Key: Hashable, Value>
28942889

28952890
/// Attach a buffer for bridged Dictionary keys.
28962891
@nonobjc
2897-
private func _initializeBridgedKeys(_ storage: _BridgingHashBuffer) -> Bool {
2898-
return _stdlib_atomicInitializeARCRef(
2899-
object: _bridgedKeysPtr,
2900-
desired: storage)
2892+
private func _initializeBridgedKeys(_ storage: _BridgingHashBuffer) {
2893+
_stdlib_atomicInitializeARCRef(object: _bridgedKeysPtr, desired: storage)
29012894
}
29022895

29032896
/// Attach a buffer for bridged Dictionary values.
29042897
@nonobjc
2905-
private func _initializeBridgedValues(
2906-
_ storage: _BridgingHashBuffer
2907-
) -> Bool {
2908-
return _stdlib_atomicInitializeARCRef(
2909-
object: _bridgedValuesPtr,
2910-
desired: storage)
2898+
private func _initializeBridgedValues(_ storage: _BridgingHashBuffer) {
2899+
_stdlib_atomicInitializeARCRef(object: _bridgedValuesPtr, desired: storage)
29112900
}
29122901

29132902
@nonobjc
@@ -2917,19 +2906,16 @@ final internal class _SwiftDeferredNSDictionary<Key: Hashable, Value>
29172906

29182907
// Allocate and initialize heap storage for bridged keys.
29192908
let bridged = _BridgingHashBuffer.allocate(
2920-
bucketCount: native._storage._bucketCount)
2909+
owner: native._storage,
2910+
hashTable: native.hashTable)
29212911
for index in native.hashTable {
29222912
let object = _bridgeAnythingToObjectiveC(native.uncheckedKey(at: index))
29232913
bridged.initialize(at: index, to: object)
29242914
}
29252915

29262916
// Atomically put the bridged keys in place.
2927-
if !_initializeBridgedKeys(bridged) {
2928-
// Lost the race.
2929-
bridged.deinitialize(elementsFrom: native.hashTable)
2930-
return _bridgedKeys!
2931-
}
2932-
return bridged
2917+
_initializeBridgedKeys(bridged)
2918+
return _bridgedKeys!
29332919
}
29342920

29352921
@nonobjc
@@ -2939,19 +2925,16 @@ final internal class _SwiftDeferredNSDictionary<Key: Hashable, Value>
29392925

29402926
// Allocate and initialize heap storage for bridged values.
29412927
let bridged = _BridgingHashBuffer.allocate(
2942-
bucketCount: native._storage._bucketCount)
2928+
owner: native._storage,
2929+
hashTable: native.hashTable)
29432930
for index in native.hashTable {
29442931
let object = _bridgeAnythingToObjectiveC(native.uncheckedValue(at: index))
29452932
bridged.initialize(at: index, to: object)
29462933
}
29472934

29482935
// Atomically put the bridged values in place.
2949-
if !_initializeBridgedValues(bridged) {
2950-
// Lost the race.
2951-
bridged.deinitialize(elementsFrom: native.hashTable)
2952-
return _bridgedValues!
2953-
}
2954-
return bridged
2936+
_initializeBridgedValues(bridged)
2937+
return _bridgedValues!
29552938
}
29562939

29572940
@usableFromInline

stdlib/public/core/Hashing.swift

Lines changed: 28 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -103,61 +103,52 @@ final internal class _SwiftEmptyNSEnumerator
103103
#if _runtime(_ObjC)
104104
/// This is a minimal class holding a single tail-allocated flat buffer,
105105
/// representing hash table storage for AnyObject elements. This is used to
106-
/// store bridged elements in deferred bridging scenarios. Lacking a _HashTable,
107-
/// instances of this class don't know which of their elements are initialized,
108-
/// so they can't be used on their own.
106+
/// store bridged elements in deferred bridging scenarios.
109107
///
110108
/// Using a dedicated class for this rather than a _HeapBuffer makes it easy to
111109
/// recognize these in heap dumps etc.
112-
internal final class _BridgingHashBuffer {
113-
internal var _bucketCount: Int
114-
115-
// This type is made with allocWithTailElems, so no init is ever called.
116-
// But we still need to have an init to satisfy the compiler.
117-
private init(_doNotUse: ()) {
118-
_sanityCheckFailure("This class cannot be directly initialized")
110+
internal final class _BridgingHashBuffer
111+
: ManagedBuffer<_BridgingHashBuffer.Header, AnyObject> {
112+
struct Header {
113+
internal var owner: AnyObject
114+
internal var hashTable: _HashTable
115+
116+
init(owner: AnyObject, hashTable: _HashTable) {
117+
self.owner = owner
118+
self.hashTable = hashTable
119+
}
119120
}
120121

121-
internal static func allocate(bucketCount: Int) -> _BridgingHashBuffer {
122-
_sanityCheck(bucketCount & (bucketCount &- 1) == 0,
123-
"Bucket count must be a power of two")
124-
let object = Builtin.allocWithTailElems_1(
125-
_BridgingHashBuffer.self,
126-
bucketCount._builtinWordValue, AnyObject.self)
127-
object._bucketCount = bucketCount
128-
return object
122+
internal static func allocate(
123+
owner: AnyObject,
124+
hashTable: _HashTable
125+
) -> _BridgingHashBuffer {
126+
let buffer = self.create(minimumCapacity: hashTable.bucketCount) { _ in
127+
Header(owner: owner, hashTable: hashTable)
128+
}
129+
return unsafeDowncast(buffer, to: _BridgingHashBuffer.self)
129130
}
130131

131132
deinit {
132-
_sanityCheck(_bucketCount == -1)
133-
}
134-
135-
internal var _elements: UnsafeMutablePointer<AnyObject> {
136-
@inline(__always) get {
137-
let ptr = Builtin.projectTailElems(self, AnyObject.self)
138-
return UnsafeMutablePointer(ptr)
133+
for index in header.hashTable {
134+
(firstElementAddress + index.bucket).deinitialize(count: 1)
139135
}
136+
_fixLifetime(self)
140137
}
141138

142139
internal subscript(index: _HashTable.Index) -> AnyObject {
143140
@inline(__always) get {
144-
_sanityCheck(index.bucket >= 0 && index.bucket < _bucketCount)
145-
return _elements[index.bucket]
141+
_sanityCheck(header.hashTable.isOccupied(index))
142+
defer { _fixLifetime(self) }
143+
return firstElementAddress[index.bucket]
146144
}
147145
}
148146

149147
@inline(__always)
150148
internal func initialize(at index: _HashTable.Index, to object: AnyObject) {
151-
_sanityCheck(index.bucket >= 0 && index.bucket < _bucketCount)
152-
(_elements + index.bucket).initialize(to: object)
153-
}
154-
155-
@inline(__always)
156-
internal func deinitialize(elementsFrom hashTable: _HashTable) {
157-
for index in hashTable {
158-
(_elements + index.bucket).deinitialize(count: 1)
159-
}
160-
_bucketCount = -1
149+
_sanityCheck(header.hashTable.isOccupied(index))
150+
(firstElementAddress + index.bucket).initialize(to: object)
151+
_fixLifetime(self)
161152
}
162153
}
163154
#endif

stdlib/public/core/Set.swift

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2156,6 +2156,7 @@ final internal class _SwiftSetNSEnumerator<Element: Hashable>
21562156
}
21572157

21582158
private func bridgedElement(at index: _HashTable.Index) -> AnyObject {
2159+
_sanityCheck(base.hashTable.isOccupied(index))
21592160
if let bridgedElements = self.bridgedElements {
21602161
return bridgedElements[index]
21612162
}
@@ -2233,10 +2234,6 @@ final internal class _SwiftDeferredNSSet<Element: Hashable>
22332234
super.init()
22342235
}
22352236

2236-
deinit {
2237-
_bridgedElements?.deinitialize(elementsFrom: native.hashTable)
2238-
}
2239-
22402237
/// Returns the pointer to the stored property, which contains bridged
22412238
/// Set elements.
22422239
@nonobjc
@@ -2256,10 +2253,8 @@ final internal class _SwiftDeferredNSSet<Element: Hashable>
22562253

22572254
/// Attach a buffer for bridged Set elements.
22582255
@nonobjc
2259-
private func _initializeBridgedElements(
2260-
_ storage: _BridgingHashBuffer
2261-
) -> Bool {
2262-
return _stdlib_atomicInitializeARCRef(
2256+
private func _initializeBridgedElements(_ storage: _BridgingHashBuffer) {
2257+
_stdlib_atomicInitializeARCRef(
22632258
object: _bridgedElementsPtr,
22642259
desired: storage)
22652260
}
@@ -2270,19 +2265,16 @@ final internal class _SwiftDeferredNSSet<Element: Hashable>
22702265

22712266
// Allocate and initialize heap storage for bridged objects.
22722267
let bridged = _BridgingHashBuffer.allocate(
2273-
bucketCount: native._storage._bucketCount)
2268+
owner: native._storage,
2269+
hashTable: native.hashTable)
22742270
for index in native.hashTable {
22752271
let object = _bridgeAnythingToObjectiveC(native.element(at: index))
22762272
bridged.initialize(at: index, to: object)
22772273
}
22782274

22792275
// Atomically put the bridged elements in place.
2280-
if !_initializeBridgedElements(bridged) {
2281-
// Lost the race.
2282-
bridged.deinitialize(elementsFrom: native.hashTable)
2283-
return _bridgedElements!
2284-
}
2285-
return bridged
2276+
_initializeBridgedElements(bridged)
2277+
return _bridgedElements!
22862278
}
22872279

22882280
@objc

0 commit comments

Comments
 (0)