From 925bee4af9890c153f736cf47845fb9dd86dfb51 Mon Sep 17 00:00:00 2001
From: Sergej Jaskiewicz <jaskiewiczs@icloud.com>
Date: Thu, 8 Jul 2021 15:35:34 +0300
Subject: [PATCH] Fix reentrancy bugs in Subscribers.Assign

---
 .../Subscribers/Subscribers.Assign.swift      | 13 +++++--
 .../Helpers/AutomaticallyFinish.swift         |  7 ++++
 .../SubscribersTests/AssignTests.swift        | 36 +++++++++++++++++++
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Sources/OpenCombine/Subscribers/Subscribers.Assign.swift b/Sources/OpenCombine/Subscribers/Subscribers.Assign.swift
index e2768df..5211e91 100644
--- a/Sources/OpenCombine/Subscribers/Subscribers.Assign.swift
+++ b/Sources/OpenCombine/Subscribers/Subscribers.Assign.swift
@@ -112,8 +112,17 @@ extension Subscribers {
             lock.assertOwner()
 #endif
             status = .terminal
-            object = nil
-            lock.unlock()
+
+            // We MUST release the object AFTER unlocking the lock,
+            // since releasing it may trigger execution of arbitrary code,
+            // for example, if the object has a deinit.
+            // When the object deallocates, its deinit is called, and holding
+            // the lock at that moment can lead to deadlocks.
+
+            withExtendedLifetime(object) {
+                object = nil
+                lock.unlock()
+            }
         }
     }
 }
diff --git a/Tests/OpenCombineTests/Helpers/AutomaticallyFinish.swift b/Tests/OpenCombineTests/Helpers/AutomaticallyFinish.swift
index f34ad87..cdee630 100644
--- a/Tests/OpenCombineTests/Helpers/AutomaticallyFinish.swift
+++ b/Tests/OpenCombineTests/Helpers/AutomaticallyFinish.swift
@@ -35,3 +35,10 @@ final class AutomaticallyFinish<Output, Failure: Error> {
                             receiveValue: receiveValue)
     }
 }
+
+extension AutomaticallyFinish where Failure == Never {
+    func assign<Root>(to keyPath: ReferenceWritableKeyPath<Root, Output>,
+                      on object: Root) -> AnyCancellable {
+        return publisher.assign(to: keyPath, on: object)
+    }
+}
diff --git a/Tests/OpenCombineTests/SubscribersTests/AssignTests.swift b/Tests/OpenCombineTests/SubscribersTests/AssignTests.swift
index 4ab7804..7d2b334 100644
--- a/Tests/OpenCombineTests/SubscribersTests/AssignTests.swift
+++ b/Tests/OpenCombineTests/SubscribersTests/AssignTests.swift
@@ -137,4 +137,40 @@ final class AssignTests: XCTestCase {
         publisher.send(100)
         XCTAssertEqual(object.value, 42)
     }
+
+    func testReceiveCompletionWhileCancelling() {
+        let cancellable: AnyCancellable
+
+        do {
+            let object = ObjectToModify()
+            cancellable = object.autofinish.assign(to: \.value, on: object)
+        }
+
+        // autofinish is deallocated here, a completion is sent to the sink
+        cancellable.cancel()
+    }
+
+    func testReceiveCompletionWhileCompleting() {
+        let cancellable: AnyCancellable
+
+        let finish: () -> Void
+
+        do {
+            let object = ObjectToModify()
+            cancellable = object.autofinish.assign(to: \.value, on: object)
+
+            let underlyingPublisher = object.autofinish.publisher
+
+            finish = { underlyingPublisher.send(completion: .finished) }
+        }
+
+        finish() // autofinish is deallocated here, a completion is sent to the sink
+
+        cancellable.cancel()
+    }
+}
+
+final class ObjectToModify {
+    let autofinish = AutomaticallyFinish<Int, Never>()
+    var value = 0
 }