[Edit: I believe this is can be closed as ‘user error’ in reading the Profiler reference counts. Suggestions on the use of GlobalScope were also provided]
It appears that coroutines retain references that prevent garbage collection even in trivial examples.
class MainActivity : AppCompatActivity() {
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setContentView(R.layout.activity_main)
GlobalScope.launch(Dispatchers.Default) {
backgroundFunction()
}
}
suspend fun backgroundFunction() {
val bitmap = Bitmap.createBitmap(4096, 4096, Bitmap.Config.ARGB_8888)
withContext(Dispatchers.Main) {
findViewById<TextView>(R.id.textView).text = "BITMAP SIZE: ${bitmap.width} x ${bitmap.height}"
}
}
}
A single Activity launches backgroundFunction, which allocates a large local Bitmap. Coroutine context then switches to the main thread to update the TextView with statistics that references bitmap.
Once backgroundFunction has finished we would expect bitmap to be available for garbage collection, but there are several references retained by the coroutine apparatus that prevent this as shown by the memory profiler:
This side-effect is hard to predict or avoid. Am I doing-it-wrong? i.e. is there a canonical way to switch context that I should be using instead that avoids this behaviour?
That issue looks like exactly the same problem. I’m alarmed that it has been known about for two years and has no responses from the Kotlin team.
If you are getting an OOM in the example then you can lower the Bitmap resolution a bit. It’s only large to make the memory leak more obvious. My real app crashes with OOM after these leaks have accumulated for a while.
You should not be be using GlobalScope to execute your coroutines as they execute outside the scope of the Activity and will not be garbage collected when the Activity is. The solution is to have the Activity implement CoRountineContext() interface. You can assign the appropriate dispatcher to the coroutineContext such as:
override val coroutineContext: CoroutineContext
get() = Dispatchers.IO + SupervisorJob()
Then when your class needs to execute a coroutine you can simply do:
` launch { someCode() }
To clean all your coroutines up so nothing leaks memory and gets gc’d when the activity does in the onStop() add the following code to clean up any coroutines that may still be running
You’re correct in that generally you should be creating a context that is specific to the activities lifecycle, but that doesn’t seem to be the cause of this situation. It’s not the activity it’s that is being leaked, but rather the bitmap created in the suspend fun.
Rupert’s expectation is that once the bitmap is no longer referenced (the block passed to withContext has been executed), it should be garbage collected regardless of whether the coroutine context has cancelled it’s children or not. If backgroundFunction() has been launched multiple times while the activity is still created each bitmap that is created should not have to wait until the activity is destroyed to be garbage collected.
I think it isn’t exactly the same problem, although solving KT-16222 would solve this one as well. The difference is that there is a much simpler solution for this problem: null-out the Continuation as soon as the coroutine is done. It is suprising to me that it doesn’t work that way already.
The coroutine doesn’t actually refer to the activity, unless explicitly so. The cancellation mechanism only requires the activity to hold a reference to the coroutine, so it can lower the isActive flag on it.
@toddburgessmedia Thanks for the information and the link, which certainly taught me things I didn’t know about coroutines. Here is the code modified as you suggest and without using GlobalScope:
class MainActivity : AppCompatActivity(), CoroutineScope {
override val coroutineContext: CoroutineContext
get() = Dispatchers.Default + SupervisorJob()
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setContentView(R.layout.activity_main)
launch {
backgroundFunction()
}
}
override fun onStop() {
super.onStop()
coroutineContext.cancelChildren()
android.util.Log.i("onStop", "STOPPED")
}
suspend fun backgroundFunction() {
val bitmap = Bitmap.createBitmap(4096, 4096,
Bitmap.Config.ARGB_8888)
withContext(Dispatchers.Main) {
findViewById<TextView>(R.id.textView).text
= "BITMAP SIZE: ${bitmap.width} x ${bitmap.height}"
}
}
}
I can confirm that if you force onStop to be called (e.g. turn the screen off or change task) then the Bitmap is garbage collected. While this is obviously an improvement, I would still expect the Bitmap to be garbage collected soon after backgroundFunction block completes, otherwise it is going to hang around and cause memory problems unless the user happens to stop the Activity.
Developers used to go to great lengths to keep track of concurrent and asynchronous tasks they launch to make sure they do not leak and to be able to cancel them. With structured concurrency of Kotlin coroutines it is no longer needed. Write the simplest code that works, and it does the right thing by design.
Perhaps I’m doing-it-wrong? How SHOULD we load a Bitmap, do something with it, then have it be garbage collected? I can’t think of a more canonical use-case for Android coroutines so there must be a recommended pattern.
At the very least, how do I call coroutineContext.cancelChildren() immediately after backgroundFunction() has finished?
Ah, sorry, I meant the implementation in the Kotlin library should do that. I don’t think it’s fixable from the outside, but the fix I describe could get into production sooner.
I cannot reproduce this problem with a self-contained example on a JVM. It seems that it has something to do with Dispatchers.Main on Android. Can it be the case that Android looper retains a reference to the last run action somewhere in its bowels? Can you show more details on how the objects gets retained there (beyond a screenshot)
I may have been too hasty to dismiss the last solution. The Android Studio Profiler doesn’t automatically call the garbage collector before dumping the heap. If I do that first then it does appear that all the references unwind and the Bitmap can be safely collected.
@rupert.rawnsley It is a bit of a hack but what about trying when you are done with the BitMap you assign it the value of null so the memory it references will no longer retain any references and making it a candidate for garbage collection
FYI when the depth column field in the heap dump is blank, it indicates the object is ready to be collected but the GC hasn’t decided to pick it up yet.
One note here: the default dispatcher you set in the coroutineContext property should be Dispatchers.Main and you should only switch context to a threadpool-backed context for blocking operations. So you should be able to write
launch {
val bitmap = withContext(IO) { Bitmap.createBitmap(...) }
findViewById<TextView>(R.id.textView).text = "BITMAP SIZE: ${bitmap.width} x ${bitmap.height}"
}
That was @toddburgessmedia suggestion, which is discussed in the article he linked to about anti-patterns. I think it stops exceptions from propagating to the parent by default when using async.
You may be right.
When using AsyncTask, anything provided to the onPostExecute would ‘leak’ until a new value was posted. We learned to never return Bitmaps, or other expensive objects, in doInBackground.
These issues may be related.
