[refactor]: improve diagnostics for escaped context instances

Author: jamieQ

Package.swift

@@ -61,21 +61,26 @@ let package = Package(
         .package(url: "https://github.com/pointfreeco/swift-macro-testing", from: "0.4.0"),
         .package(url: "https://github.com/pointfreeco/swift-perception", "1.5.0" ..< "3.0.0"),
         .package(url: "https://github.com/pointfreeco/swift-custom-dump", from: "1.3.3"),
+        // 'xctest-dynamic-overlay' is actually for "IssueReporting"
         .package(url: "https://github.com/pointfreeco/xctest-dynamic-overlay", from: "1.0.0"),
     ],
     targets: [
         // MARK: Workflow
 
         .target(
             name: "Workflow",
-            dependencies: ["ReactiveSwift"],
+            dependencies: [
+                .product(name: "IssueReporting", package: "xctest-dynamic-overlay"),
+                .product(name: "ReactiveSwift", package: "ReactiveSwift"),
+            ],
             path: "Workflow/Sources"
         ),
         .target(
             name: "WorkflowTesting",
             dependencies: [
                 "Workflow",
                 .product(name: "CustomDump", package: "swift-custom-dump"),
+                .product(name: "IssueReporting", package: "xctest-dynamic-overlay"),
             ],
             path: "WorkflowTesting/Sources",
             linkerSettings: [.linkedFramework("XCTest")]
@@ -125,7 +130,10 @@ let package = Package(
 
         .target(
             name: "WorkflowReactiveSwift",
-            dependencies: ["ReactiveSwift", "Workflow"],
+            dependencies: [
+                "Workflow",
+                .product(name: "ReactiveSwift", package: "ReactiveSwift"),
+            ],
             path: "WorkflowReactiveSwift/Sources"
         ),
         .target(
@@ -139,7 +147,10 @@ let package = Package(
 
         .target(
             name: "WorkflowRxSwift",
-            dependencies: ["RxSwift", "Workflow"],
+            dependencies: [
+                "Workflow",
+                .product(name: "RxSwift", package: "RxSwift"),
+            ],
             path: "WorkflowRxSwift/Sources"
         ),
         .target(

Workflow/Sources/Debugging.swift

@@ -14,6 +14,10 @@
  * limitations under the License.
  */
 
+#if DEBUG
+import IssueReporting
+#endif
+
 public struct WorkflowUpdateDebugInfo: Codable, Equatable {
     public var workflowType: String
     public var kind: Kind
@@ -168,3 +172,30 @@ extension WorkflowUpdateDebugInfo {
         )
     }()
 }
+
+// MARK: - Runtime Debugging Utilities
+
+#if DEBUG
+/// Debug facility that checks if an instance of a reference type may have 'escaped' from a function.
+/// - Parameters:
+///   - object: The instance to test.
+///   - message: The message to log if not uniquely referenced. The ObjectIdentifier of the instance will be supplied as an argument.
+///
+/// If the instance is **not** known to be uniquely referenced it will:
+///   - Trigger a test failure if running in a testing context.
+///   - Cause an assertion failure otherwise.
+func diagnoseEscapedReference(
+    to object: consuming some AnyObject,
+    _ message: (ObjectIdentifier) -> String
+) {
+    var maybeUniqueReference = consume object
+    if !isKnownUniquelyReferenced(&maybeUniqueReference) {
+        if let _ = TestContext.current {
+            reportIssue(message(ObjectIdentifier(maybeUniqueReference)))
+        } else {
+            assertionFailure(message(ObjectIdentifier(maybeUniqueReference)))
+        }
+    }
+    _ = consume maybeUniqueReference
+}
+#endif

Workflow/Sources/SubtreeManager.swift

@@ -16,6 +16,10 @@
 
 import Dispatch
 
+#if DEBUG
+import IssueReporting
+#endif
+
 extension WorkflowNode {
     /// Manages the subtree of a workflow. Specifically, this type encapsulates the logic required to update and manage
     /// the lifecycle of nested workflows across multiple render passes.
@@ -76,6 +80,16 @@ extension WorkflowNode {
 
             wrapped.invalidate()
 
+            #if DEBUG
+            // Try to ensure it didn't escape from the invocation.
+            diagnoseEscapedReference(to: consume wrapped) { objectID in
+                "Detected escape of a RenderContext<\(WorkflowType.self)> instance from a `render()` method. A RenderContext is only valid to use within render(); allowing it to escape may lead to incorrect behavior. Search for the address of '\(objectID)' in the memory graph debugger to determine why it may have escaped."
+            }
+            #else
+            // For binding lifetime consistency
+            _ = consume wrapped
+            #endif
+
             /// After the render is complete, assign children using *only the children that were used during the render
             /// pass.* This means that any pre-existing children that were *not* used during the render pass are removed
             /// as a result of this call to `render`.

Workflow/Sources/WorkflowNode.swift

@@ -249,7 +249,6 @@ extension WorkflowNode {
         do {
             // FIXME: can we avoid instantiating a class here somehow?
             let context = ConcreteApplyContext(storage: workflow)
-            defer { context.invalidate() }
             let wrappedContext = ApplyContext.make(implementation: context)
 
             let renderOnlyIfStateChanged = hostContext.runtimeConfig.renderOnlyIfStateChanged
@@ -302,6 +301,21 @@ extension WorkflowNode {
             } else {
                 result = performSimpleActionApplication(markStateAsChanged: true)
             }
+
+            // N.B. This logic would perhaps be nicer to put in a defer
+            // statement, but that seems to mess with the precise control
+            // we need over binding lifetimes to perform these checks.
+            _ = consume wrappedContext
+            context.invalidate()
+            #if DEBUG
+            // Try to ensure it didn't escape from the invocation.
+            diagnoseEscapedReference(to: consume context) { objectID in
+                "Detected escaped reference to an ApplyContext<\(A.WorkflowType.self)> when applying action '\(action)'. An ApplyContext is only valid for the duration of its associated action application, and should not escape. Search for the address of '\(objectID)' in the memory graph debugger to investigate."
+            }
+            #else
+            // For binding lifetime consistency
+            _ = consume context
+            #endif
         }
 
         return result

Workflow/Tests/ApplyContextTests.swift

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import IssueReporting
 import Testing
 
 @testable import Workflow
@@ -38,7 +39,10 @@ struct ApplyContextTests {
         let emitEvent = node.render()
         node.enableEvents()
 
-        emitEvent()
+        // We're intentionally escaping the context
+        withExpectedIssue {
+            emitEvent()
+        }
 
         #expect(escapedContext != nil)
         #expect(escapedContext?.concreteStorage == nil)

Workflow/Tests/SubtreeManagerTests.swift

@@ -14,8 +14,10 @@
  * limitations under the License.
  */
 
+import IssueReporting
 import ReactiveSwift
 import XCTest
+
 @testable import Workflow
 
 final class SubtreeManagerTests: XCTestCase {
@@ -126,14 +128,17 @@ final class SubtreeManagerTests: XCTestCase {
 
         var escapingContext: RenderContext<ParentWorkflow>!
 
-        _ = manager.render { context -> TestViewModel in
-            XCTAssertTrue(context.isValid)
-            escapingContext = context
-            return context.render(
-                workflow: TestWorkflow(),
-                key: "",
-                outputMap: { _ in AnyWorkflowAction.noAction }
-            )
+        // We expect the context escape check to detect this
+        withExpectedIssue {
+            _ = manager.render { context -> TestViewModel in
+                XCTAssertTrue(context.isValid)
+                escapingContext = context
+                return context.render(
+                    workflow: TestWorkflow(),
+                    key: "",
+                    outputMap: { _ in AnyWorkflowAction.noAction }
+                )
+            }
         }
         manager.enableEvents()