SingletonImageLoader.Factory newImageLoader‘s parameter should be application context #3213
Description
Describe the bug
A clear description of what the bug is. Enable logging and attach any stack traces. You can also check the onError parameter of AsyncImage/SubcomposeAsyncImage/rememberAsyncImagePainter.
/**
* Create and set the new singleton [ImageLoader].
*/
private fun newImageLoader(context: PlatformContext): ImageLoader {
// Local storage to ensure newImageLoader is invoked at most once.
var imageLoader: ImageLoader? = null
return reference.updateAndGet { value ->
when {
value is ImageLoader -> value
imageLoader != null -> imageLoader
else -> {
((value as? Factory)?.newImageLoader(context)
?: context.applicationImageLoaderFactory()?.newImageLoader(context)
?: DefaultSingletonImageLoaderFactory.newImageLoader(context))
.also { imageLoader = it }
}
}
} as ImageLoader
}Currently, the context parameter provided by newImageLoader comes from the first place where ImageLoader is used. In the vast majority of applications, this context should be an Activity. However, in the ImageLoader.Builder, many functions use lazy initialization. This means that if the context parameter from newImageLoader is used, memory leaks can occur unknowingly.
Such as:
override fun newImageLoader(context: PlatformContext): ImageLoader {
return ImageLoader.Builder(this)
.diskCache {
DiskCache.Builder()
.directory(context.getExternalFilesDir("coil")!!.toOkioPath())
.build()
}
.logger(if (isDebuggable) DebugLogger() else null)
.build()
}memory leak trace:
┬───
│ GC Root: Thread object
│
├─ android.net.ConnectivityThread instance
│ Leaking: NO (PathClassLoader↓ is not leaking)
│ Thread name: 'ConnectivityThread'
│ ↓ Thread.contextClassLoader
├─ dalvik.system.PathClassLoader instance
│ Leaking: NO (SingletonImageLoader↓ is not leaking and A ClassLoader is never leaking)
│ ↓ ClassLoader.runtimeInternalObjects
├─ java.lang.Object[] array
│ Leaking: NO (SingletonImageLoader↓ is not leaking)
│ ↓ Object[3124]
├─ coil3.SingletonImageLoader class
│ Leaking: NO (a class is never leaking)
│ ↓ static SingletonImageLoader.reference
│ ~~~~~~~~~
├─ java.util.concurrent.atomic.AtomicReference instance
│ Leaking: UNKNOWN
│ Retaining 768.8 kB in 8847 objects
│ ↓ AtomicReference.value
│ ~~~~~
├─ coil3.RealImageLoader instance
│ Leaking: UNKNOWN
│ Retaining 768.8 kB in 8846 objects
│ ↓ RealImageLoader.options
│ ~~~~~~~
├─ coil3.RealImageLoader$Options instance
│ Leaking: UNKNOWN
│ Retaining 767.1 kB in 8737 objects
│ application instance of com.***.***.App
│ ↓ RealImageLoader$Options.diskCacheLazy
│ ~~~~~~~~~~~~~
├─ kotlin.SynchronizedLazyImpl instance
│ Leaking: UNKNOWN
│ Retaining 766.6 kB in 8721 objects
│ ↓ SynchronizedLazyImpl.initializer
│ ~~~~~~~~~~~
├─ com.***.***.App$$ExternalSyntheticLambda2 instance
│ Leaking: UNKNOWN
│ Retaining 766.6 kB in 8720 objects
│ f$0 instance of com.***.***.**.****.****Activity with mDestroyed = true
│ ↓ App$$ExternalSyntheticLambda2.f$0
│ ~~~
╰→ com.***.***.**.****.****Activity instance
Leaking: YES (ObjectWatcher was watching this because com.***.***.**.****.****Activity
received Activity#onDestroy() callback and Activity#mDestroyed is true)
Retaining 766.6 kB in 8719 objects
key = fce22fae-a5e3-4f70-838a-59775a69bb0b
watchDurationMillis = 5575
retainedDurationMillis = 575
mApplication instance of com.***.***.App
mBase instance of androidx.appcompat.view.ContextThemeWrapper
The DiskCacheLazy implicitly holds a reference to the context object, leading to memory leaks. Although, in general, the Factory interface is implemented by the Application class, and developers can use an instance of the Application to do this, but it is still a relatively easy trap to fall into.
To Reproduce
How can we reproduce this? Please attach a code snippet or small project that reproduces the bug. If possible, include any related image assets. Bug reports without reproduction steps are likely to be closed.
Implement the SingletonImageLoader.Factory interface using the Application class and use the code I provided. Then, load an image in any activity and exit the activity before the image loading is complete to reproduce the issue.
Version
What Coil version are you using? Does this occur on a specific API level or Android emulator/device?
Any version.