google / ExoPlayer

This project is deprecated and stale. The latest ExoPlayer code is available in https://github.com/androidx/media
https://developer.android.com/media/media3/exoplayer
Apache License 2.0
21.7k stars 6.02k forks source link

ExoPlayer set up with Rtmp extension leaks memory when closing the activity with playerView #4249

Closed AdamGrzybkowski closed 5 years ago

AdamGrzybkowski commented 6 years ago

Issue description

LeakCanary detects a leak when closing activity with PlayerView.

I'm using the ExoPlayer with Rtmp extensions to display the live stream. When the url is invalid (there is no live stream there) the leak occurs.

This is how the releasePlayer method looks like

private fun releasePlayer() {
        player?.release()
        player = null
        playerView.player = null
        mediaSource = null
        trackSelector = null
}

screenshot_20180514-081829

Version of ExoPlayer being used

2.8.0

ojw28 commented 6 years ago

There are two parts to this issue:

  1. The leak occurs because RtmpClient.open blocks forever when an attempt is made to connect to an invalid address, rather than failing. This is what stops the thread right at the top of the chain from exiting, and is the root cause. This is a bug in LibRtmp rather than ExoPlayer, which I've reported here: https://github.com/ant-media/LibRtmp-Client-for-Android/issues/67.
  2. We could mitigate the leak by nulling ExtractorMediaPeriod.callback when ExtractorMediaPeriod.release() is called. It's probably a good idea to do this for all other MediaPeriod implementations as well, particularly given that loader threads may not exit immediately even when they're working correctly (e.g. whilst LibRtmp shouldn't block forever, it's not unreasonable that it might end up blocking until some timeout is reached).
ojw28 commented 6 years ago

We've made some changes to ExoPlayer that will mitigate the leak. The MediaPeriod will still leak, which is unavoidable until the root cause is fixed in LibRtmp. However we now explicitly null references from that point, to allow GC of ExoPlayerImplInternal, the Activity and so on. This is a good thing to do from a robustness point of view, and may allow faster GC for some other use cases too.

Leaving the issue open to track picking up a newer version of LibRtmp when the root cause is addressed.

AdamGrzybkowski commented 6 years ago

Great @ojw28 thanks :)