From 38377dfe0c54e4ce0d261a9e46c5ca7a307a8f6b Mon Sep 17 00:00:00 2001 From: Chris Amanse Date: Wed, 4 Dec 2019 14:34:10 -0800 Subject: [PATCH 1/5] Fix memory leaks in ThreadBarriers.swift --- .../ThreadBarriers.swift | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift b/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift index 9f5e183447021..b6ec6856ca77a 100644 --- a/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift +++ b/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift @@ -69,20 +69,29 @@ public func _stdlib_thread_barrier_init( InitializeConditionVariable(barrier.pointee.cond!) #else barrier.pointee.mutex = UnsafeMutablePointer.allocate(capacity: 1) - if pthread_mutex_init(barrier.pointee.mutex!, nil) != 0 { - // FIXME: leaking memory. - return -1 - } barrier.pointee.cond = UnsafeMutablePointer.allocate(capacity: 1) - if pthread_cond_init(barrier.pointee.cond!, nil) != 0 { - // FIXME: leaking memory, leaking a mutex. - return -1 + guard _stdlib_pthread_barrier_mutex_and_cond_init(barrier) == 0 else { + barrier.pointee.mutex!.deinitialize(count: 1) + barrier.pointee.mutex!.deallocate() + barrier.pointee.cond!.deinitialize(count: 1) + barrier.pointee.cond!.deallocate() } #endif barrier.pointee.count = count return 0 } +private func _stdlib_pthread_barrier_mutex_and_cond_init(_ barrier: UnsafeMutablePointer<_stdlib_pthread_barrier_t>) -> CInt { + guard pthread_mutex_init(barrier.pointee.mutex!) == 0 else { + return -1 + } + guard pthread_cond_init(barrier.pointee.cond!) == 0 else { + pthread_mutex_destroy(barrier.pointee.mutex!) + return -1 + } + return 0 +} + public func _stdlib_thread_barrier_destroy( _ barrier: UnsafeMutablePointer<_stdlib_thread_barrier_t> ) -> CInt { @@ -90,12 +99,8 @@ public func _stdlib_thread_barrier_destroy( // Condition Variables do not need to be explicitly destroyed // Mutexes do not need to be explicitly destroyed #else - if pthread_cond_destroy(barrier.pointee.cond!) != 0 { - // FIXME: leaking memory, leaking a mutex. - return -1 - } - if pthread_mutex_destroy(barrier.pointee.mutex!) != 0 { - // FIXME: leaking memory. + guard pthread_cond_destroy(barrier.pointee.cond!) == 0 && + pthread_mutex_destroy(barrier.pointee.mutex!) == 0 else { return -1 } #endif From 315f51062539e9433161e408e965b8e5c5592285 Mon Sep 17 00:00:00 2001 From: Chris Amanse Date: Wed, 4 Dec 2019 14:44:27 -0800 Subject: [PATCH 2/5] Fatal error on pthread cond/mutex destroy failure --- .../SwiftPrivateThreadExtras/SwiftPrivateThreadExtras.swift | 5 +---- .../private/SwiftPrivateThreadExtras/ThreadBarriers.swift | 6 +++--- validation-test/stdlib/StringSlicesConcurrentAppend.swift | 3 +-- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/stdlib/private/SwiftPrivateThreadExtras/SwiftPrivateThreadExtras.swift b/stdlib/private/SwiftPrivateThreadExtras/SwiftPrivateThreadExtras.swift index 002aef8e74145..bc849536a9429 100644 --- a/stdlib/private/SwiftPrivateThreadExtras/SwiftPrivateThreadExtras.swift +++ b/stdlib/private/SwiftPrivateThreadExtras/SwiftPrivateThreadExtras.swift @@ -162,10 +162,7 @@ public class _stdlib_Barrier { } deinit { - let ret = _stdlib_thread_barrier_destroy(_threadBarrierPtr) - if ret != 0 { - fatalError("_stdlib_thread_barrier_destroy() failed") - } + _stdlib_thread_barrier_destroy(_threadBarrierPtr) } public func wait() { diff --git a/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift b/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift index b6ec6856ca77a..00b53dfa75e6e 100644 --- a/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift +++ b/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift @@ -94,14 +94,14 @@ private func _stdlib_pthread_barrier_mutex_and_cond_init(_ barrier: UnsafeMutabl public func _stdlib_thread_barrier_destroy( _ barrier: UnsafeMutablePointer<_stdlib_thread_barrier_t> -) -> CInt { +) { #if os(Windows) // Condition Variables do not need to be explicitly destroyed // Mutexes do not need to be explicitly destroyed #else guard pthread_cond_destroy(barrier.pointee.cond!) == 0 && pthread_mutex_destroy(barrier.pointee.mutex!) == 0 else { - return -1 + fatalError("_stdlib_thread_barrier_destroy() failed") } #endif barrier.pointee.cond!.deinitialize(count: 1) @@ -110,7 +110,7 @@ public func _stdlib_thread_barrier_destroy( barrier.pointee.mutex!.deinitialize(count: 1) barrier.pointee.mutex!.deallocate() - return 0 + return } public func _stdlib_thread_barrier_wait( diff --git a/validation-test/stdlib/StringSlicesConcurrentAppend.swift b/validation-test/stdlib/StringSlicesConcurrentAppend.swift index 64441e7a184ea..942449e9dfdc8 100644 --- a/validation-test/stdlib/StringSlicesConcurrentAppend.swift +++ b/validation-test/stdlib/StringSlicesConcurrentAppend.swift @@ -111,8 +111,7 @@ StringTestSuite.test("SliceConcurrentAppend") { expectEqual(0, joinRet1) expectEqual(0, joinRet2) - ret = _stdlib_thread_barrier_destroy(barrierVar!) - expectEqual(0, ret) + _stdlib_thread_barrier_destroy(barrierVar!) barrierVar!.deinitialize(count: 1) barrierVar!.deallocate() From 9e80f27466030035d33b409850c717ed24141729 Mon Sep 17 00:00:00 2001 From: Chris Amanse Date: Mon, 9 Dec 2019 14:38:54 -0800 Subject: [PATCH 3/5] Rename pthread to thread --- stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift b/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift index 00b53dfa75e6e..c689e620dfa5d 100644 --- a/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift +++ b/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift @@ -70,7 +70,7 @@ public func _stdlib_thread_barrier_init( #else barrier.pointee.mutex = UnsafeMutablePointer.allocate(capacity: 1) barrier.pointee.cond = UnsafeMutablePointer.allocate(capacity: 1) - guard _stdlib_pthread_barrier_mutex_and_cond_init(barrier) == 0 else { + guard _stdlib_thread_barrier_mutex_and_cond_init(barrier) == 0 else { barrier.pointee.mutex!.deinitialize(count: 1) barrier.pointee.mutex!.deallocate() barrier.pointee.cond!.deinitialize(count: 1) @@ -81,7 +81,7 @@ public func _stdlib_thread_barrier_init( return 0 } -private func _stdlib_pthread_barrier_mutex_and_cond_init(_ barrier: UnsafeMutablePointer<_stdlib_pthread_barrier_t>) -> CInt { +private func _stdlib_thread_barrier_mutex_and_cond_init(_ barrier: UnsafeMutablePointer<_stdlib_thread_barrier_t>) -> CInt { guard pthread_mutex_init(barrier.pointee.mutex!) == 0 else { return -1 } From f0e31eef243f99356006966a9afa015ef28a51b2 Mon Sep 17 00:00:00 2001 From: Chris Amanse Date: Tue, 10 Dec 2019 14:37:26 -0800 Subject: [PATCH 4/5] Fix pthread init function calls --- stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift b/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift index c689e620dfa5d..c30bec861aadb 100644 --- a/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift +++ b/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift @@ -82,10 +82,10 @@ public func _stdlib_thread_barrier_init( } private func _stdlib_thread_barrier_mutex_and_cond_init(_ barrier: UnsafeMutablePointer<_stdlib_thread_barrier_t>) -> CInt { - guard pthread_mutex_init(barrier.pointee.mutex!) == 0 else { + guard pthread_mutex_init(barrier.pointee.mutex!, nil) == 0 else { return -1 } - guard pthread_cond_init(barrier.pointee.cond!) == 0 else { + guard pthread_cond_init(barrier.pointee.cond!, nil) == 0 else { pthread_mutex_destroy(barrier.pointee.mutex!) return -1 } From c1e6cf8932fa8bd29e4fb8595ac6a26a54f5ee2b Mon Sep 17 00:00:00 2001 From: Chris Amanse Date: Wed, 11 Dec 2019 12:05:32 -0800 Subject: [PATCH 5/5] Fix guard statement --- stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift b/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift index c30bec861aadb..6d79bd124e976 100644 --- a/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift +++ b/stdlib/private/SwiftPrivateThreadExtras/ThreadBarriers.swift @@ -75,6 +75,7 @@ public func _stdlib_thread_barrier_init( barrier.pointee.mutex!.deallocate() barrier.pointee.cond!.deinitialize(count: 1) barrier.pointee.cond!.deallocate() + return -1 } #endif barrier.pointee.count = count