In principle, it is fine. However, I have a couple of comments about the given implementation.
The first is that once you create a Task, it keeps running until it has produced a result or is cancelled. From the documentation:
Tasks can start running immediately after creation; you don’t explicitly start or schedule them. After creating a task, you use the instance to interact with it — for example, to wait for it to complete or to cancel it. It’s not a programming error to discard a reference to a task without waiting for that task to finish or canceling it. A task runs regardless of whether you keep a reference to it. However, if you discard the reference to a task, you give up the ability to wait for that task’s result or cancel the task.
Since these tasks are executing an infinite loop, every time a Model object is instantiated, you create and leak three tasks.
Now, SwiftUI has a particular way of managing the lifetimes of observable objects, so it tries to not repeatedly instantiate @StateObject values if your view's identity does not change, but:
- If your view's identity does change (for example, because it is used in a
ForEach), even @StateObject values will be recreated, and
- Not all code uses SwiftUI's particular way of managing object lifetimes. It is good engineering practice to clean up any resources you allocate, rather than leak them.
We can simulate this. Here, I've set things up really simply - I've renamed your ContentView to ChildView, and created two of them with explicit IDs. There's a button which changes their IDs.
struct ContentView: View {
@State var firstID = 0
@State var secondID = 1
var body: some View {
VStack {
GroupBox {
ChildView()
.id(firstID)
.border(.black)
ChildView()
.id(secondID)
.border(.black)
}
Button {
firstID += 2
secondID += 2
} label: {
Text("Change IDs")
}
}.padding()
}
}
struct ChildView: View {
@StateObject private var model = Model()
var body: some View {
VStack {
Button("Red") {
model.postRed()
}
Button("Green") {
model.postGreen()
}
Button("Blue") {
model.postBlue()
}
}.padding()
}
}
In Model, we print some logging information in init before we create the tasks, and again inside each worker task when it is about to complete.
@MainActor
class Model: ObservableObject {
// ...
init() {
print("Creating tasks") // <----
Task {
defer { print("Worker task ending") } // <----
for await _ in NotificationCenter.default.notifications(named: redNotificationName) {
await taskRed()
}
}
// ...
}
}
When I launch the application, I find that it prints "Creating tasks" twice - one for each Model object. That's expected. And when I click the button to change IDs, it prints "Creating tasks" twice again - because that's how @StateObject works. But - here's the critical thing - we never see "Worker task ending". Even worse, if we change IDs a couple of times to create a bunch of tasks, then click "Red" one time, all of those tasks (which are still running) get triggered and all print to the console.
So how do we fix this? The last sentence of the documentation snippet tells us - we need Model to keep references to the tasks it creates in init, so it can cancel them in its deinit and stop them living forever:
@MainActor
class Model: ObservableObject {
let redNotificationName = Notification.Name("red")
let greenNotificationName = Notification.Name("green")
let blueNotificationName = Notification.Name("blue")
var red = 0
var green = 0
var blue = 0
var tasks = [Task<Void, Never>]() // <----
init() {
print("Creating tasks")
tasks.append( // <-----
Task {
defer { print("Worker task ending") }
for await _ in NotificationCenter.default.notifications(named: redNotificationName) {
await taskRed()
}
}
)
// ... same for the other 2 tasks
}
deinit {
tasks.forEach { $0.cancel() } // <----
}
}
What is interesting is that even if you run this, our worker tasks still won't get cleaned up. What gives?
Basically, what is happening is that in order for our tasks to access properties such as redNotificationName, or invoke callbacks such as taskRed(), they need to keep the instance of Model alive which they belong to. But then, that instance of Model also has a reference to the worker task. In other words, we have a retain cycle.
Typically, Swift guards against retain cycles by having you explicitly write self in order to access this data which prolongs the object's lifetime, but Task { ... } has a special exception which allows it to omit that. The reason is that Task { ... } is intended to be used with actors; when you call it from within an actor, it inherits the actor's context, and in that particular situation no lifetime extension actually occurs.
But in this case, we're not calling it from within an actor, and so it does extend the lifetime of our Model instance and create a retain cycle. It is discussed a bit more in this thread: Task.detached doesn’t allow implicit self
The solution is to use Task.detached { ... } instead of plain Task { ... }. If we make that one tiny change, the compiler suddenly and requires us to be explicit when we extend the lifetime of the Model object:
tasks.append(
Task.detached {
defer { print("Worker task ending") }
for await _ in NotificationCenter.default.notifications(named: redNotificationName) {
// ^
// ERROR: Reference to property 'redNotificationName' in closure requires explicit use of 'self' to make capture semantics explicit
await taskRed()
// ^
// ERROR: Implicit use of 'self' in closure; use 'self.' to make capture semantics explicit
}
}
)
In general, explicit self is a really nice feature of Swift, and it's a bit of a shame that the compiler lets us make this mistake so easily.
In order to break the cycle, we can use a weak or unowned reference to self. Since the task will be cancelled in the object's deinit, this code should never even try to access properties self after the object's lifetime has ended, so I'm going to use an unowned reference:
tasks.append(
Task.detached { [unowned self] in
defer { print("Worker task ending") }
for await _ in NotificationCenter.default.notifications(named: redNotificationName) {
await taskRed()
}
}
)
And that's it! We're done. Now if we click the "Change IDs" button, we can see that 2 new Model objects are created, which causes the 2 previous objects to be destroyed (there's no retain cycle keeping them alive any more), which causes them to each cancel their 3 worker tasks. We're not leaking any tasks any more.
And as a final test - if we click "Change IDs" a bunch of times to remake the Model objects, then click "Red", we can see that there are only 2 worker tasks alive listening to notifications (one per Model object):
To summarise, 2 changes were needed:
-
Hold on to the Task references in an array, use them to cancel the tasks in deinit
-
Create the tasks using Task.detached { ... } rather than Task { ... }, and use weak/unowned captures of self to avoid the retain cycle