Skip to content

Commit 3fec0ee

Browse files
authored
[fix]: use weak reference to internal sinks when vending to clients (#189)
previously we would allow a strong reference to the internal `ReusableSink` to be captured by the `Sink` types that are vended to clients. this meant that events sent into such 'external' sinks would still propagate through the event handling machinery a bit before realizing that the backing workflow node to which they were destined was no longer around. by using weak references we can prevent extending the life of the internal sinks (and underlying EventPipe) unnecessarily in such cases. - make `ReusableSink` captures `weak` when vending to the 'outside world' - update various access control values to facilitate testing this behavior
1 parent 3787a94 commit 3fec0ee

File tree

2 files changed

+26
-7
lines changed

2 files changed

+26
-7
lines changed

Workflow/Sources/SubtreeManager.swift

+6-7
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ extension WorkflowNode {
2323
internal var onUpdate: ((Output) -> Void)?
2424

2525
/// Sinks from the outside world (i.e. UI)
26-
private var eventPipes: [EventPipe] = []
26+
internal private(set) var eventPipes: [EventPipe] = []
2727

2828
/// Reusable sinks from the previous render pass
2929
private var previousSinks: [ObjectIdentifier: AnyReusableSink] = [:]
@@ -198,12 +198,11 @@ extension WorkflowNode.SubtreeManager {
198198
func makeSink<Action>(of actionType: Action.Type) -> Sink<Action> where Action: WorkflowAction, WorkflowType == Action.WorkflowType {
199199
let reusableSink = sinkStore.findOrCreate(actionType: Action.self)
200200

201-
let signpostRef = SignpostRef()
201+
let sink = Sink<Action> { [weak reusableSink] action in
202+
WorkflowLogger.logSinkEvent(ref: SignpostRef(), action: action)
202203

203-
let sink = Sink<Action> { action in
204-
WorkflowLogger.logSinkEvent(ref: signpostRef, action: action)
205-
206-
reusableSink.handle(action: action)
204+
// use a weak reference as we'd like control over the lifetime
205+
reusableSink?.handle(action: action)
207206
}
208207

209208
return sink
@@ -291,7 +290,7 @@ extension WorkflowNode.SubtreeManager {
291290
// MARK: - EventPipe
292291

293292
extension WorkflowNode.SubtreeManager {
294-
fileprivate final class EventPipe {
293+
internal final class EventPipe {
295294
var validationState: ValidationState
296295
enum ValidationState {
297296
case preparing

Workflow/Tests/SubtreeManagerTests.swift

+20
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,26 @@ final class SubtreeManagerTests: XCTestCase {
243243
XCTAssertEqual(manager.sideEffectLifetimes.count, 1)
244244
XCTAssertEqual(manager.sideEffectLifetimes.keys.first, "key-2")
245245
}
246+
247+
func test_eventPipes_notRetainedByExternalSinks() {
248+
weak var weakEventPipe: WorkflowNode<TestWorkflow>.SubtreeManager.EventPipe?
249+
var externalSink: Sink<TestWorkflow.Event>?
250+
autoreleasepool {
251+
let manager = WorkflowNode<TestWorkflow>.SubtreeManager()
252+
253+
manager.render { context in
254+
externalSink = context.makeSink(of: TestWorkflow.Event.self)
255+
}
256+
257+
weakEventPipe = manager.eventPipes.last
258+
259+
XCTAssertEqual(manager.eventPipes.count, 1)
260+
XCTAssertNotNil(weakEventPipe)
261+
}
262+
263+
XCTAssertNotNil(externalSink)
264+
XCTAssertNil(weakEventPipe)
265+
}
246266
}
247267

248268
private struct TestViewModel {

0 commit comments

Comments
 (0)