Skip to content

Commit 96a7610

Browse files
committed
Add comprehensive memory leak tests for PassthroughRelay
- Test subscription release with different initialization orders - Test withLatestFrom operator scenarios - Mirrors test coverage from CurrentValueRelay (PR #137) - Verifies fix for issue #167
1 parent 85aae44 commit 96a7610

File tree

2 files changed

+205
-3
lines changed

2 files changed

+205
-3
lines changed

Sources/Relays/PassthroughRelay.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ private extension PassthroughRelay {
7878
}
7979

8080
func forceFinish() {
81-
self.sink?.shouldForwardCompletion = true
82-
self.sink?.receive(completion: .finished)
83-
self.sink = nil
81+
sink?.shouldForwardCompletion = true
82+
sink?.receive(completion: .finished)
83+
sink = nil
8484
}
8585

8686
func request(_ demand: Subscribers.Demand) {

Tests/PassthroughRelayTests.swift

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,5 +143,207 @@ class PassthroughRelayTests: XCTestCase {
143143
XCTAssertFalse(completed)
144144
XCTAssertEqual(values, ["initial", "1", "2", "3"])
145145
}
146+
147+
// MARK: - Memory Leak Tests (Issue #167, PR #168)
148+
149+
// There was a race condition which caused subscriptions in PassthroughRelay
150+
// to leak. Details of the race condition are in this PR:
151+
//
152+
// https://github.com/CombineCommunity/CombineExt/pull/168
153+
//
154+
// The issue is similar to CurrentValueRelay (PR #137). The easiest way to
155+
// reproduce the race condition is to initialize `cancellables` before `relay`.
156+
// These tests confirm subscriptions are properly released regardless of
157+
// initialization order.
158+
159+
final class StoredObject {
160+
nonisolated(unsafe) static var storedObjectReleased = false
161+
162+
let value = 10
163+
164+
init() {
165+
Self.storedObjectReleased = false
166+
}
167+
168+
deinit {
169+
Self.storedObjectReleased = true
170+
}
171+
}
172+
173+
final class StoredObject2 {
174+
nonisolated(unsafe) static var storedObjectReleased = false
175+
176+
let value = 20
177+
178+
init() {
179+
Self.storedObjectReleased = false
180+
}
181+
182+
deinit {
183+
Self.storedObjectReleased = true
184+
}
185+
}
186+
187+
func testSubscriptionIsReleasedWhenRelayIsDeallocatedAndDeclaredAfterCancellables() {
188+
final class ContainerClass {
189+
nonisolated(unsafe) static var receivedCompletion = false
190+
nonisolated(unsafe) static var receivedCancel = false
191+
192+
// Cancellables comes before the relay
193+
var cancellables = Set<AnyCancellable>()
194+
let relay = PassthroughRelay<StoredObject>()
195+
196+
init() {
197+
relay
198+
.handleEvents(receiveCancel: {
199+
Self.receivedCancel = true
200+
})
201+
.sink(
202+
receiveCompletion: { _ in
203+
Self.receivedCompletion = true
204+
},
205+
receiveValue: { _ in }
206+
)
207+
.store(in: &cancellables)
208+
}
209+
}
210+
211+
var container: ContainerClass? = ContainerClass()
212+
213+
XCTAssertFalse(ContainerClass.receivedCompletion)
214+
XCTAssertFalse(ContainerClass.receivedCancel)
215+
container = nil
216+
XCTAssertNil(container)
217+
218+
// Cancellables is deallocated before the relay, so cancel is called
219+
XCTAssertFalse(ContainerClass.receivedCompletion)
220+
XCTAssertTrue(ContainerClass.receivedCancel)
221+
}
222+
223+
func testSubscriptionIsReleasedWhenRelayIsDeallocatedAndDeclaredBeforeCancellables() {
224+
final class ContainerClass {
225+
nonisolated(unsafe) static var receivedCompletion = false
226+
nonisolated(unsafe) static var receivedCancel = false
227+
228+
// Relay comes before cancellables
229+
let relay = PassthroughRelay<StoredObject>()
230+
var cancellables = Set<AnyCancellable>()
231+
232+
init() {
233+
relay
234+
.handleEvents(receiveCancel: {
235+
Self.receivedCancel = true
236+
})
237+
.sink(
238+
receiveCompletion: { _ in
239+
Self.receivedCompletion = true
240+
},
241+
receiveValue: { _ in }
242+
)
243+
.store(in: &cancellables)
244+
}
245+
}
246+
247+
var container: ContainerClass? = ContainerClass()
248+
249+
XCTAssertFalse(ContainerClass.receivedCompletion)
250+
XCTAssertFalse(ContainerClass.receivedCancel)
251+
container = nil
252+
XCTAssertNil(container)
253+
254+
// Relay is deallocated first, so completion is sent
255+
XCTAssertTrue(ContainerClass.receivedCompletion)
256+
XCTAssertFalse(ContainerClass.receivedCancel)
257+
}
258+
259+
func testStoredObjectsAreReleasedWithWithLatestFromAndDeclaredBeforeCancellables() {
260+
final class ContainerClass {
261+
nonisolated(unsafe) static var receivedCompletion = false
262+
nonisolated(unsafe) static var receivedCancel = false
263+
264+
// Relays come before cancellables
265+
let relay = PassthroughRelay<StoredObject>()
266+
let relay2 = PassthroughRelay<StoredObject2>()
267+
var cancellables: Set<AnyCancellable>? = Set<AnyCancellable>()
268+
269+
init() {
270+
relay
271+
.withLatestFrom(relay2)
272+
.handleEvents(receiveCancel: {
273+
Self.receivedCancel = true
274+
})
275+
.sink(
276+
receiveCompletion: { _ in
277+
Self.receivedCompletion = true
278+
},
279+
receiveValue: { _ in }
280+
)
281+
.store(in: &cancellables!)
282+
283+
// Send initial values so withLatestFrom has something to work with.
284+
relay2.accept(StoredObject2())
285+
}
286+
}
287+
288+
var container: ContainerClass? = ContainerClass()
289+
290+
XCTAssertFalse(ContainerClass.receivedCompletion)
291+
XCTAssertFalse(StoredObject.storedObjectReleased)
292+
XCTAssertFalse(StoredObject2.storedObjectReleased)
293+
294+
container = nil
295+
XCTAssertTrue(StoredObject2.storedObjectReleased)
296+
XCTAssertNil(container)
297+
298+
// withLatestFrom keeps the relay subscription alive until cancellables are released.
299+
XCTAssertFalse(ContainerClass.receivedCompletion)
300+
XCTAssertTrue(ContainerClass.receivedCancel)
301+
}
302+
303+
func testStoredObjectsAreReleasedWithWithLatestFromAndDeclaredAfterCancellables() {
304+
final class ContainerClass {
305+
nonisolated(unsafe) static var receivedCompletion = false
306+
nonisolated(unsafe) static var receivedCancel = false
307+
308+
// Cancellables comes before the relays - this is the problematic case
309+
var cancellables: Set<AnyCancellable>? = Set<AnyCancellable>()
310+
let relay = PassthroughRelay<StoredObject>()
311+
let relay2 = PassthroughRelay<StoredObject2>()
312+
313+
init() {
314+
relay
315+
.withLatestFrom(relay2)
316+
.handleEvents(receiveCancel: {
317+
Self.receivedCancel = true
318+
})
319+
.sink(
320+
receiveCompletion: { _ in
321+
Self.receivedCompletion = true
322+
},
323+
receiveValue: { _ in }
324+
)
325+
.store(in: &cancellables!)
326+
327+
// Send initial values so withLatestFrom has something to work with.
328+
relay2.accept(StoredObject2())
329+
}
330+
}
331+
332+
var container: ContainerClass? = ContainerClass()
333+
334+
XCTAssertFalse(ContainerClass.receivedCompletion)
335+
XCTAssertFalse(StoredObject.storedObjectReleased)
336+
XCTAssertFalse(StoredObject2.storedObjectReleased)
337+
338+
// Setting container to nil deallocates cancellables first
339+
// This should not crash and should properly release objects
340+
container = nil
341+
XCTAssertTrue(StoredObject2.storedObjectReleased)
342+
XCTAssertNil(container)
343+
344+
// Cancellables deallocated first, so cancel is called
345+
XCTAssertFalse(ContainerClass.receivedCompletion)
346+
XCTAssertTrue(ContainerClass.receivedCancel)
347+
}
146348
}
147349
#endif

0 commit comments

Comments
 (0)