I recently stumble upon a weird memory leak situation that involved a Task and closure, and I can't seem to wrap my head around it.
Consider the following class:
class MyClass {
var myClosure: (() -> Void)?
deinit {
print("deinit MyClass")
}
}
I have a view controller that init an instance of MyClass in viewDidLoad().
class MyViewController: UIViewController {
var mc: MyClass!
override func viewDidLoad() {
super.viewDidLoad()
mc = MyClass()
mc.myClosure = {
Task { [weak self] in
// Do something with weak self
}
}
}
deinit {
print("deinit MyViewController")
}
}
Now, if I present MyViewController and then dismiss it, both MyViewController and MyClass are not being deallocated. What is even weird is that if I remove the [weak self] capture in the Task, then both of them will be deallocated.
So my questions are:
Why did the memory leak occur?
Why removing [weak self] will fix the memory leak?
Yes, adding [weak self] to both closures fixed the memory leak. I think your thesis kind of makes sense. Or maybe a better way to put it is: The outer closure automatically captures self strongly so that the inner closure can capture self weakly.
Meaning this entire situation has nothing to do with Task, it is all about capturing self in closures.
As a general rule, it is rarely necessary or helpful to use weak captures in a Task closure, since tasks already don't form permanent references to their context; like any other function, they will retain the values they use only for as long as the code executing needs them, and when the task ends, it will release any remaining local state.
Along similar lines, you can avoid the need to use weak references in closures if you manually discard them when they're no longer needed. If MyClass.myClosure represents a one-time callback, for instance, then you could reset it to nil after you invoke the closure, which will also break the reference cycle.
Yes, rarely, but just be aware that there are cases where capturing self weakly is essential. For example, when using a Task with an infinite AsyncSequence. Doing so allows us to break out of the for-await loop when self is deallocated.
Task { [weak self] in
let asyncSequence = NotificationCenter.default.notifications(
named: UIDevice.orientationDidChangeNotification
)
for await _ in asyncSequence {
guard let self else {
// Break out of the loop
return
}
}
}
I think it would be best practice to cancel the whole thing rather than letting observations like this run for the rest of the lifetime of your program (even if you guard and return early).
Correct me if I'm wrong, but isn't the [weak self] approach better than manually canceling the task, as it eliminates the possibility of human errors, such as forgetting to cancel the task?
I don't think so, because you have lingering tasks observing notifications. That's memory being used, that's cpu being woken up every time a notification fires even though you do nothing with it. It might not be very noticeable but it's not very clean (to me).
Some time ago we encountered the same issue. Double weak solves the problem.
To me it looks like an expected behavior because you are accessing self directly when using [weak self] in the Task so you are capturing the strong reference. If you imagine that [weak self] does something like: let optionalSelf = WeakObject(self) then it becomes more clear why this issue occurs.
@tclmentdev How about Combine? It looks like in this case the [weak] in receiveValue closure is essential, isn't it?
.sink { [weak self] _ in
Task { [weak self] in await self?.update() }
}
I think it's the same. If you properly cancel your subscription then you don't need to capture weakly. It's all equivalent to what Joe was saying above here about resetting a closure to nil and not having to capture things weakly.
Sounds like a headache if you want to subscribe to something for the whole lifetime of let's say ViewModel.
This way you would have to track when you leave the screen, then you would have to differentiate whether it's forward or backward navigation, etc. Much simpler to just add a weak and less error-prone.
Yes, I think SwiftUI has tools for this (e.g. onDisappear). And if you do not do this, as you use your app over time, you would accumulate more and more dead observations that do nothing. Another solution might be to use weaks but still unsubscribe/cancel from the deinit.
To be fair, the documentation for the Task type does not mention this, nor does the documentation for any of the Task initialisers (where those closures are provided).
In other contexts, if you initialise a type by providing an @escaping closure, the assumption is that the closure's lifetime will match that of the initialised value (e.g. the value will save the closure as a stored property). If in this case the lifetime of the closure is truly independent of the lifetime of the created Task handle, then that fact should be a documented guarantee. Otherwise, developers should assume that it is an implementation detail and not rely on it, IMO.
Isn't it just how Tasks work? Once a Task is finished, its instance is released. Therefore, it implies that the closure will be released as well. I don't see any undocumented facts here.
But it doesn't guarantee that the Task will be released once the parent object is not needed anymore. An example where you can see that the object will be released only when the Task is done:
import Foundation
import PlaygroundSupport
PlaygroundPage.current.needsIndefiniteExecution = true
class SampleObject {
let value: String
init(value: String) {
self.value = value
print("init")
Task {
try? await Task.sleep(for: .seconds(2))
print(self.value)
}
}
deinit {
print("deinit")
}
}
weak var _: SampleObject? = SampleObject(value: "old value")
print("the object is still alive")
By adding [weak self] in the Task we break the strong reference and let the SampleObject to be released sooner.
Therefore, it is as helpful as in any other place where you use closures. If you have a long running Task you may not want it to keep a reference to the parent object. For example, if you call API inside a Task, most likely you don't want your ViewModel to be alive for the next 10 seconds if the request is stuck and you already left the screen.
It is not obvious that a Task releases any resources when it is finished executing because it does not document that it does so. "Isn't it just how Tasks work?" is simply your own understanding based on how Task happens to be implemented today.
For example, we might want Task to store its closure indefinitely, perhaps in order to support a .retry() operation (as an example). If we were to make that change, we would not have broken any API contracts.
Fundamentally, the @escaping attribute does not specify for how long the closure's lifetime is being extended. Will this code create an ownership cycle? Will that cycle be temporary or permanent?
We have no way of representing that information in the type system, and it is clearly important to ensure that clients can use the API with confidence, therefore we must communicate it via documentation.
This is an internal thing how it is implemented. You won't be able to implement retry or anything relying on the stored closure, because the Task doesn't give you access to the closure. And that's exactly the reason why most likely it is not documented. Unless you are talking about the documentation for Swift contributors' purposes.
@escaping attribute does tell you that the object capturing this closure may keep it for its whole lifetime or shorter. And that's what is exactly happening in the case of Task.
Once the task is finished the closure is released because it is no longer needed and there are no public operations that could call it again. Therefore, keeping this closure would be a waste of resources.
Probably, most classes that use @escaping closures are releasing them when no longer needed. This is a common practice.
What's the example in which a lack of knowledge that the closure has been released earlier than the Task handle causes some issues?
Going further, Task is a struct, therefore if you keep a copy of it, it wouldn't be able to release the closure (even using mutating function, because we keep a different copy). Example:
Therefore, the closure should be still alive if you keep a copy of a Task, but it's not. So it implies that the closure is not even kept by the Task, it is most likely pushed on some queue and discarded once it's processed.
I feel you may be misunderstanding my point - whatever happens should be documented. Documentation forms part of an API contract.
Any experiments, however interesting, do not replace the need to specify which behaviour developers should expect of a correctly functioning standard library implementation.
I guess this thread became an offtopic :D. We have 3 things being discussed here:
Why we need external [weak self] here:
myClosure = { [weak self] in
Task { [weak self] in
// Do something with weak self
}
}
Whether it makes sens to use [weak self] inside a Task
If it's needed to document that the closure captured by the Task may be released earlier than the Task handle itself
Regarding the last point, the more documentation the better, unless it becomes overwhelming :D. However, my point was that the current behavior doesn't break anything and it's expected that the memory is released when no longer needed, so it would be just additional information.
