Skip to content

Commit 9103eb0

Browse files
author
Gleb Tarasov
committed
Avoid deallocations of the view controllers on background threads
If the view controller is deallocated on the background thread, it can cause the data races in UIKit. Trying to avoid that.
1 parent 2dbbe22 commit 9103eb0

File tree

3 files changed

+42
-3
lines changed

3 files changed

+42
-3
lines changed

PerformanceSuite/Sources/ViewControllerObserver.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,16 @@ final class ViewControllerObserverFactory<T: ViewControllerObserver, S: ScreenMe
3333
private let metricsReceiver: S
3434
private let observerMaker: (S.ScreenIdentifier) -> T
3535

36+
private func ensureDeallocationOnTheMainThread(viewController: UIViewController) {
37+
DispatchQueue.main.async {
38+
// Make sure viewController is deallocated on the main thread, because
39+
// if the last access is on the background thread, it will be deallocated
40+
// in background, and it can cause data races in UIKit.
41+
// Calling `hash` to make sure this call is not removed by the compilation optimizer
42+
_ = viewController.hash
43+
}
44+
}
45+
3646
private func observer(for viewController: UIViewController) -> T? {
3747
if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
3848
dispatchPrecondition(condition: .onQueue(PerformanceMonitoring.queue))
@@ -66,6 +76,7 @@ final class ViewControllerObserverFactory<T: ViewControllerObserver, S: ScreenMe
6676
if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
6777
PerformanceMonitoring.queue.async {
6878
self.observer(for: viewController)?.beforeInit(viewController: viewController)
79+
self.ensureDeallocationOnTheMainThread(viewController: viewController)
6980
}
7081
} else {
7182
observer(for: viewController)?.beforeInit(viewController: viewController)
@@ -76,6 +87,7 @@ final class ViewControllerObserverFactory<T: ViewControllerObserver, S: ScreenMe
7687
if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
7788
PerformanceMonitoring.queue.async {
7889
self.observer(for: viewController)?.beforeViewDidLoad(viewController: viewController)
90+
self.ensureDeallocationOnTheMainThread(viewController: viewController)
7991
}
8092
} else {
8193
observer(for: viewController)?.beforeViewDidLoad(viewController: viewController)
@@ -86,6 +98,7 @@ final class ViewControllerObserverFactory<T: ViewControllerObserver, S: ScreenMe
8698
if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
8799
PerformanceMonitoring.queue.async {
88100
self.observer(for: viewController)?.afterViewDidAppear(viewController: viewController)
101+
self.ensureDeallocationOnTheMainThread(viewController: viewController)
89102
}
90103
} else {
91104
observer(for: viewController)?.afterViewDidAppear(viewController: viewController)
@@ -96,6 +109,7 @@ final class ViewControllerObserverFactory<T: ViewControllerObserver, S: ScreenMe
96109
if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
97110
PerformanceMonitoring.queue.async {
98111
self.observer(for: viewController)?.beforeViewWillDisappear(viewController: viewController)
112+
self.ensureDeallocationOnTheMainThread(viewController: viewController)
99113
}
100114
} else {
101115
observer(for: viewController)?.beforeViewWillDisappear(viewController: viewController)
@@ -106,6 +120,7 @@ final class ViewControllerObserverFactory<T: ViewControllerObserver, S: ScreenMe
106120
if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
107121
PerformanceMonitoring.queue.async {
108122
self.observer(for: viewController)?.afterViewWillAppear(viewController: viewController)
123+
self.ensureDeallocationOnTheMainThread(viewController: viewController)
109124
}
110125
} else {
111126
observer(for: viewController)?.afterViewWillAppear(viewController: viewController)
@@ -116,6 +131,7 @@ final class ViewControllerObserverFactory<T: ViewControllerObserver, S: ScreenMe
116131
if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
117132
PerformanceMonitoring.queue.async {
118133
self.observer(for: viewController)?.beforeViewDidDisappear(viewController: viewController)
134+
self.ensureDeallocationOnTheMainThread(viewController: viewController)
119135
}
120136
} else {
121137
observer(for: viewController)?.beforeViewDidDisappear(viewController: viewController)

PerformanceSuite/Tests/TTIObserverExtensionTests.swift

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class TTIObserverExtensionTests: XCTestCase {
1919

2020
override func setUpWithError() throws {
2121
try super.setUpWithError()
22+
PerformanceMonitoring.experiments = Experiments(observersOnBackgroundQueue: true)
2223
defaultTimeProvider = timeProvider
2324
metricsReceiver = TTIMetricsReceiverStub()
2425
try PerformanceMonitoring.enable(config: [.screenLevelTTI(metricsReceiver)], experiments: Experiments(observersOnBackgroundQueue: true))
@@ -244,7 +245,8 @@ class TTIObserverExtensionTests: XCTestCase {
244245
window.rootViewController = tabbar
245246
window.makeKeyAndVisible()
246247

247-
waitForExpectations(timeout: 3, handler: nil)
248+
wait(for: [exp1], timeout: 3)
249+
PerformanceMonitoring.queue.sync {}
248250

249251
// for the first controller we should calculate TTI between `init` and `screenIsReady` -> 1 second
250252
vc1.screenIsReady()
@@ -334,7 +336,8 @@ class TTIObserverExtensionTests: XCTestCase {
334336
window.rootViewController = vc1
335337
window.makeKeyAndVisible()
336338

337-
waitForExpectations(timeout: 3, handler: nil)
339+
wait(for: [exp1], timeout: 3)
340+
PerformanceMonitoring.queue.sync {}
338341

339342
// we call screenIsReady for the child, but it should work for the parent
340343
vc2.screenIsReady()
@@ -403,6 +406,10 @@ private class MyViewController: UIViewController {
403406
super.viewDidLayoutSubviews()
404407
output += "viewDidLayoutSubviews\n"
405408
}
409+
410+
deinit {
411+
XCTAssertTrue(Thread.isMainThread)
412+
}
406413
}
407414

408415
private struct MyView: View {

PerformanceSuite/Tests/ViewControllerObserverTests.swift

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,18 @@ import XCTest
1212

1313
class ViewControllerObserverTests: XCTestCase {
1414

15+
override func setUp() {
16+
super.setUp()
17+
PerformanceMonitoring.experiments = Experiments(observersOnBackgroundQueue: true)
18+
}
19+
20+
override func tearDown() {
21+
super.tearDown()
22+
PerformanceMonitoring.consumerQueue.sync { }
23+
PerformanceMonitoring.queue.sync { }
24+
PerformanceMonitoring.experiments = Experiments()
25+
}
26+
1527
func testObserversCollection() {
1628
let o1 = Observer()
1729
let o2 = Observer()
@@ -159,4 +171,8 @@ private class MetricsConsumerForSwiftUITest: ScreenMetricsReceiver {
159171
func appRenderingMetricsReceived(metrics: RenderingMetrics) {}
160172
}
161173

162-
private class MyViewController: UIViewController { }
174+
private class MyViewController: UIViewController {
175+
deinit {
176+
XCTAssertTrue(Thread.isMainThread)
177+
}
178+
}

0 commit comments

Comments
 (0)