Disposables Can Cause Memory Leaks

02 Feb 2021
•
3 min read

Every Disposable holds a strong reference to the observer it binds. This can lead to surprising memory leaks.

Consider the following example:

class TacoViewModel : ViewModel() {

  var compositeDisposable = CompositeDisposable()

  fun loadTaco(activity: Activity) {
    compositeDisposable.add(
      Single.just(Taco())
        .subscribe { taco ->
          // Handle taco...
          println("Taco created in $activity")
        }
    )
  }

  override fun onCleared() {
    compositeDisposable.clear()
  }
}

Simple, right? This is a classic pattern for avoiding memory leaks in RxJava code: keep the returned Disposable and dispose/clear it in the “end” event of whatever scope you’re in. This is great if you want to prevent memory leaks if your TacoViewModel lifecycle ends before the Single terminates.

However, this actually leaks activities anyway because even though they’re disposed, we keep Disposable instances around until onCleared() is called.

Congratulations, you now have a good old-fashioned Android Activity memory leak. Drop this into your app, load some tacos, rotate, and watch LeakCanary start to chirp 🐤.

    ┬───
    │ GC Root: System class
    │
   /// ...
    │  
    ├─ autodispose2.sample.TacoViewModel instance
    │    Leaking: UNKNOWN
    │    Retaining 289.4 kB in 7953 objects
    │    ↓ TacoViewModel.compositeDisposable
    │                    ~~~~~~~~~~~~~~~~~~~
    ├─ io.reactivex.rxjava3.disposables.CompositeDisposable instance
    │    Leaking: UNKNOWN
    │    Retaining 289.3 kB in 7949 objects
    │    ↓ CompositeDisposable.resources
    │                          ~~~~~~~~~
    ├─ io.reactivex.rxjava3.internal.util.OpenHashSet instance
    │    Leaking: UNKNOWN
    │    Retaining 289.3 kB in 7948 objects
    │    ↓ OpenHashSet.keys
    │                  ~~~~
    ├─ java.lang.Object[] array
    │    Leaking: UNKNOWN
    │    Retaining 289.3 kB in 7947 objects
    │    ↓ Object[].[0]
    │               ~~~
    ├─ io.reactivex.rxjava3.internal.observers.ConsumerSingleObserver instance
    │    Leaking: UNKNOWN
    │    Retaining 36 B in 2 objects
    │    ↓ ConsumerSingleObserver.onSuccess
    │                             ~~~~~~~~~
    ├─ autodispose2.sample.TacoViewModel$loadTaco$1 instance
    │    Leaking: UNKNOWN
    │    Retaining 16 B in 1 objects
    │    Anonymous class implementing io.reactivex.rxjava3.functions.Consumer
    │    $activity instance of autodispose2.sample.HomeActivity with mDestroyed =
    │    true
    │    ↓ TacoViewModel$loadTaco$1.$activity
    │                               ~~~~~~~~~
    ╰→ autodispose2.sample.HomeActivity instance
    ​     Leaking: YES (ObjectWatcher was watching this because autodispose2.sample.
    ​     HomeActivity received Activity#onDestroy() callback and
    ​     Activity#mDestroyed is true)
    ​     Retaining 144.8 kB in 3973 objects

Springing the Leak

In our example, the Consumer lambda passed to subscribe() is what’s called a capturing lambda, because it retains a reference to the original activity. The returned Disposable, in turn, keeps a reference to this consumer. You can actually see this directly in the leak canary trace, where the disposable is actually the ConsumerSingleObserver it refs:

    ├─ io.reactivex.rxjava3.internal.observers.ConsumerSingleObserver instance
    │    Leaking: UNKNOWN
    │    Retaining 36 B in 2 objects
    │    ↓ ConsumerSingleObserver.onSuccess
    │                             ~~~~~~~~~
    ├─ autodispose2.sample.TacoViewModel$loadTaco$1 instance
    │    Leaking: UNKNOWN
    │    Retaining 16 B in 1 objects
    │    Anonymous class implementing io.reactivex.rxjava3.functions.Consumer
    │    $activity instance of autodispose2.sample.HomeActivity with mDestroyed =
    │    true
    │    ↓ TacoViewModel$loadTaco$1.$activity
    │                               ~~~~~~~~~

This instance lives on forever inside the CompositeDisposable until onCleared() is called. This means every activity passed into loadTaco() is leaked, even if temporarily, until onCleared is called.

Even though we’ve added what looks like proper disposal in onCleared(), our retained Disposable instance is, itself, a leak because it’s still transitively holding onto a reference to anything captured in the lambda!

This can happen with any Disposable too. CompositeDisposable exacerbates this further because it will accumulate these potential leaks.

“I don’t use ViewModel and nothing in our codebase outlives Activity, do I need to think about this?”

Yes! The example above uses a simple ViewModel because I’m aware of my Android-centric audience :). This isn’t unique to them however, or even to Android. Replace ViewModel with whatever construct you want (Presenter, Manager, etc) and Activity with anything you don’t want to leak.

@Singleton
class HttpClient {
  val compositeDisposable = CompositeDisposable()
  
  fun request(url: String, callback: Callback) {
    // Leaks every callback and everything it 
    // references unless you call shutdown() 🙃
    compositeDisposable.add(makeRequestSingle(url)
        .subscribe { callback.onResponse(it) })
  }
  
  fun cancelRequests() {
    compositeDisposable.clear()
  }
}

class MyPresenter {
  var disposable: Disposable? = null
  
  fun bind(context: Context) {
    // Leaks context until onStop()
    disposable = Single.just(1)
        .subscribe { println(context) } 
  }
  
  fun onStop() {
    disposable?.dispose()
    // Persists even after this unless you 
    // discard your Presenter or null out the disposable
    disposable = null
  }
}

…and so on and so forth. Disposable holds on to everything captured in your observer/consumer, regardless of the context and whether or not you’ve called dispose() on it.

Solutions

• If you’re keeping the returned Disposable, plan to do something with it or discard it. Don’t just dispose() it when needed but rather also clear your reference to it when the stream terminates if possible. You could explore writing your own WeakLambdaObserver or something similar, too.
• Avoid capturing lambdas where possible, but these happen easily. Would be neat if there was a lint check to warn about these 🤔.
• Save yourself some time and use AutoDispose (disclaimer: I’m the original author). AutoDispose doesn’t avoid this exact issue per se, but it does eliminate the need to keep the returned Disposable around 99% of the time.

Special thanks to Py and Dan for reviewing this.