Skip to content

Commit ffa6bf4

Browse files
authored
Fix memory leaks in ThreadBarriers.swift (#12212)
* Fix memory leaks in ThreadBarriers.swift * Fatal error on pthread cond/mutex destroy failure * Rename pthread to thread * Fix pthread init function calls * Fix guard statement
1 parent dd09401 commit ffa6bf4

File tree

3 files changed

+23
-21
lines changed

3 files changed

+23
-21
lines changed

stdlib/private/SwiftPrivateThreadExtras/SwiftPrivateThreadExtras.swift

+1-4
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,7 @@ public class _stdlib_Barrier {
162162
}
163163

164164
deinit {
165-
let ret = _stdlib_thread_barrier_destroy(_threadBarrierPtr)
166-
if ret != 0 {
167-
fatalError("_stdlib_thread_barrier_destroy() failed")
168-
}
165+
_stdlib_thread_barrier_destroy(_threadBarrierPtr)
169166
}
170167

171168
public func wait() {

stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift

+21-15
Original file line numberDiff line numberDiff line change
@@ -69,34 +69,40 @@ public func _stdlib_thread_barrier_init(
6969
InitializeConditionVariable(barrier.pointee.cond!)
7070
#else
7171
barrier.pointee.mutex = UnsafeMutablePointer.allocate(capacity: 1)
72-
if pthread_mutex_init(barrier.pointee.mutex!, nil) != 0 {
73-
// FIXME: leaking memory.
74-
return -1
75-
}
7672
barrier.pointee.cond = UnsafeMutablePointer.allocate(capacity: 1)
77-
if pthread_cond_init(barrier.pointee.cond!, nil) != 0 {
78-
// FIXME: leaking memory, leaking a mutex.
73+
guard _stdlib_thread_barrier_mutex_and_cond_init(barrier) == 0 else {
74+
barrier.pointee.mutex!.deinitialize(count: 1)
75+
barrier.pointee.mutex!.deallocate()
76+
barrier.pointee.cond!.deinitialize(count: 1)
77+
barrier.pointee.cond!.deallocate()
7978
return -1
8079
}
8180
#endif
8281
barrier.pointee.count = count
8382
return 0
8483
}
8584

85+
private func _stdlib_thread_barrier_mutex_and_cond_init(_ barrier: UnsafeMutablePointer<_stdlib_thread_barrier_t>) -> CInt {
86+
guard pthread_mutex_init(barrier.pointee.mutex!, nil) == 0 else {
87+
return -1
88+
}
89+
guard pthread_cond_init(barrier.pointee.cond!, nil) == 0 else {
90+
pthread_mutex_destroy(barrier.pointee.mutex!)
91+
return -1
92+
}
93+
return 0
94+
}
95+
8696
public func _stdlib_thread_barrier_destroy(
8797
_ barrier: UnsafeMutablePointer<_stdlib_thread_barrier_t>
88-
) -> CInt {
98+
) {
8999
#if os(Windows)
90100
// Condition Variables do not need to be explicitly destroyed
91101
// Mutexes do not need to be explicitly destroyed
92102
#else
93-
if pthread_cond_destroy(barrier.pointee.cond!) != 0 {
94-
// FIXME: leaking memory, leaking a mutex.
95-
return -1
96-
}
97-
if pthread_mutex_destroy(barrier.pointee.mutex!) != 0 {
98-
// FIXME: leaking memory.
99-
return -1
103+
guard pthread_cond_destroy(barrier.pointee.cond!) == 0 &&
104+
pthread_mutex_destroy(barrier.pointee.mutex!) == 0 else {
105+
fatalError("_stdlib_thread_barrier_destroy() failed")
100106
}
101107
#endif
102108
barrier.pointee.cond!.deinitialize(count: 1)
@@ -105,7 +111,7 @@ public func _stdlib_thread_barrier_destroy(
105111
barrier.pointee.mutex!.deinitialize(count: 1)
106112
barrier.pointee.mutex!.deallocate()
107113

108-
return 0
114+
return
109115
}
110116

111117
public func _stdlib_thread_barrier_wait(

validation-test/stdlib/StringSlicesConcurrentAppend.swift

+1-2
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ StringTestSuite.test("SliceConcurrentAppend") {
111111
expectEqual(0, joinRet1)
112112
expectEqual(0, joinRet2)
113113

114-
ret = _stdlib_thread_barrier_destroy(barrierVar!)
115-
expectEqual(0, ret)
114+
_stdlib_thread_barrier_destroy(barrierVar!)
116115

117116
barrierVar!.deinitialize(count: 1)
118117
barrierVar!.deallocate()

0 commit comments

Comments
 (0)