Fix memory unsafety

MaxDesiatov · Sep 23, 2021 · 21f4781 · 21f4781
1 parent 1307205
commit 21f4781
Showing 1 changed file with 49 additions and 42 deletions.
diff --git a/Tests/OpenCombineTests/Helpers/ExecuteOnBackgroundThread.swift b/Tests/OpenCombineTests/Helpers/ExecuteOnBackgroundThread.swift
@@ -20,19 +20,23 @@ import WinSDK
 // We could use Foundation's Thread, but it doesn't work on Linux for some
 // reason.
 
-private class Thread<ResultType> {
+private class JoinThread<ResultType> {
+
+    private typealias ThreadRoutine = () -> UnsafeMutableRawPointer
 
 #if canImport(Darwin)
     private typealias ThreadHandle = UnsafeMutablePointer<pthread_t?>
+    private typealias Context = ThreadRoutine
 #elseif canImport(Glibc)
     private typealias ThreadHandle = UnsafeMutablePointer<pthread_t>
+    private typealias Context = (ThreadRoutine, ResultPtr)
 #elseif os(Windows)
     private typealias ThreadHandle = HANDLE?
+    private typealias Context = (ThreadRoutine, ResultPtr)
 #endif
 
-    private typealias ThreadRoutine = () -> UnsafeMutableRawPointer
-
     private var handle: ThreadHandle
+    private var context: UnsafeMutablePointer<Context> = .allocate(capacity: 1)
 
 #if os(Windows)
     private typealias ResultPtr = UnsafeMutablePointer<UnsafeMutableRawPointer>
@@ -51,62 +55,65 @@ private class Thread<ResultType> {
 
 #if os(Windows)
         handle = nil
-        typealias Context = (ThreadRoutine, ResultPtr)
-        var context = (typeErasedBody, resultPtr)
+        context.initialize(to: (typeErasedBody, resultPtr))
 #else
         handle = .allocate(capacity: 1)
-        var context = typeErasedBody
+        context.initialize(to: typeErasedBody)
 #endif
 
-        withUnsafeMutablePointer(to: &context) { context in
 #if os(Windows)
-            print("creating thread")
-            handle = CreateThread(
-                nil, // default security attributes
-                0, // use default stack size
-                { context in
-                    let (typeErasedBody, resultPtr) = context!
-                        .assumingMemoryBound(to: Context.self)
-                        .pointee
-
-                    resultPtr.initialize(to: typeErasedBody())
-                    return 0
-                },
-                context,
-                0, // use default creation flags
-                nil // don't return thread identifier
-            )
-            print("created thread \(handle)")
-            precondition(handle != nil, "Could not create a thread")
+        print("creating thread")
+        handle = CreateThread(
+            nil, // default security attributes
+            0, // use default stack size
+            { context in
+                let (typeErasedBody, resultPtr) = context!
+                    .assumingMemoryBound(to: Context.self)
+                    .pointee
+
+                resultPtr.initialize(to: typeErasedBody())
+                return 0
+            },
+            context,
+            0, // use default creation flags
+            nil // don't return thread identifier
+        )
+        print("created thread \(handle)")
+        precondition(handle != nil, "Could not create a thread")
 #else
-            let status = pthread_create(
-                handle,
-                nil,
-                { context in
+        let status = pthread_create(
+            handle,
+            nil,
+            { context in
 #if canImport(Darwin)
-                    let context = context
+                let context = context
 #else
-                    let context = context!
+                let context = context!
 #endif
-                    return context
-                        .assumingMemoryBound(to: ThreadRoutine.self)
-                        .pointee()
-                },
-                context
-            )
-
-            precondition(status == 0, "Could not create a thread")
+                return context
+                    .assumingMemoryBound(to: ThreadRoutine.self)
+                    .pointee()
+            },
+            context
+        )
+
+        precondition(status == 0, "Could not create a thread")
 #endif
-        }
     }
 
     deinit {
+        if !joined {
+            fatalError("The thread must be joined!")
+        }
 #if os(Windows)
         CloseHandle(handle!)
+        // resultPtr is already deinitialized!
         resultPtr.deallocate()
 #else
         handle.deallocate()
 #endif
+        context.deinitialize(count: 1)
+        context.deallocate()
     }
 
     func join() -> ResultType {
@@ -115,7 +122,7 @@ private class Thread<ResultType> {
 #if os(Windows)
         print("waiting for thread to finish")
         WaitForSingleObject(handle, INFINITE)
-        print("thread finished")
+        print("")
         return resultPtr.pointee.assumingMemoryBound(to: ResultType.self).move()
 #else
 #if canImport(Darwin)
@@ -143,7 +150,7 @@ private class Thread<ResultType> {
 func executeOnBackgroundThread<ResultType>(
     _ body: () -> ResultType
 ) -> ResultType {
-    return withoutActuallyEscaping(body) { Thread($0).join() }
+    return withoutActuallyEscaping(body) { JoinThread($0).join() }
 }
 
 #endif // !WASI