Application using the library will crash if 1000 DownloadHelper instances are created with a non-null RenderersFactory

Version

Media3 1.3.0

More version details

d13a0f4

Devices that reproduce the issue

All devices

Devices that do not reproduce the issue

No devices excluded

Reproducible in the demo app?

No

Reproduction steps

Create a player application that can create and release 1000 DownloadHelper instances with a non-null RenderersFactory (or modify the demo app to do so.)
Create and release 1000 DownloadHelper instances with a non-null RenderersFactory

Expected result

An application using the player should be able to create an unlimited number of DownloadHelper instances with a non-null RenderersFactory.

Actual result

The application will crash with a "Too many receivers, total of 1000, registered for pid" exception.

Source of Issue
The source of the issue is that the DownloadHelper uses the passed RenderersFactory to create renderers, and to get capabilities for those renderers, but never releases the renderers that are created. One of the renderers created is a MediaCodecAudioRenderer. That renderer is created with a DefaultAudioSink. When the DownloadHelper processes a DOWNLOAD_HELPER_CALLBACK_MESSAGE_PREPARED message, the DownloadHelper calls runTrackSelection and this results in the DefaultAudioSink creating an instance of AudioCapabilitiesReceiver and registering that receiver. The release() method for the DefaultAudioSink unregisters that listener, but since the DownloadHelper does not call release() on the renderers it creates, release is never called on the DefaultAudioSink and the unregister never occurs. If 1000 DownloadHelpers are created with a non-null RenderersFactory, the application will crash with a "Too many receivers, total of 1000, registered for pid" exception.

Reproduction with Demo App
Not reproducible with the demo app only because there are not 1000 items to download. However, the registration of a AudioCapabilitiesReceiver for each download without a corresponding unregister can be seen with the demo app. Set breakpoints at lines 1520 audioCapabilitiesReceived.unregister(); and 1749 audioCapabilities = audioCapabilitiesReceiver.register(); in DefaultAudioSink and initiated a download with the demo app. You will hit the breakpoint at 1749 (register) for each download, but the breakpoint at 1520 (unregister) is never hit. This demonstrates receivers being registered but never unregistered. Note that the player does properly release the renderers that it creates. It is only the DownloadHelper that does not.

Recommend Change
The DownloadHelper needs to keep a handle to the renderers it creates much in the same way it keeps a handle on the RendererCapabilities that it obtains. When DownloadHelper.release() is called, the class needs to iterate through the renderers that it has created and call the release method for each. This will result the MediaCodecAudioRenderer being released, which will in turn result in the the DefaultAudioSink being released and the AudioCapabilitiesReceiver being unregistered.

Media

Not applicable

Bug Report

You will email the zip file produced by adb bugreport to android-media-github@google.com after filing this issue.

2. Create and release 1000 DownloadHelper instances with a non-null RenderersFactory

I tried to repro this by adding a for (int i = 0; i < 1001; i++) loop around this code:

media/demos/main/src/main/java/androidx/media3/demo/main/DownloadTracker.java

Lines 120 to 128 in 66547cc

    
             if (startDownloadDialogHelper != null) { 
        
               startDownloadDialogHelper.release(); 
        
             } 
        
             startDownloadDialogHelper = 
        
                 new StartDownloadDialogHelper( 
        
                     fragmentManager, 
        
                     DownloadHelper.forMediaItem(context, mediaItem, renderersFactory, dataSourceFactory), 
        
                     mediaItem); 
        
           }

This should result in creating, preparing, then releasing 1001 DownloadHelper instances.

I then launched the application and downloaded the Misc > Dizzy (MP4) asset.

The application didn't crash. I also added checkNotNull to ensure that renderersFactory is non-null.

I then tried waiting for DownloadHelper.Callback.onPrepared before releasing and re-creating, by adding the following code here:

media/demos/main/src/main/java/androidx/media3/demo/main/DownloadTracker.java

Line 196 in 66547cc

public void onPrepared(DownloadHelper helper) {

if (prepareCount.getAndIncrement() < 2001) {
  downloadHelper.release();
  downloadHelper =
      DownloadHelper.forMediaItem(context, mediaItem, renderersFactory, dataSourceFactory);
  downloadHelper.prepare(this);
}

The application still didn't crash.

Please can you provide some more precise repro steps, ideally as a fork of the demo app that we can clone.

I will work on modifying the demo app to reproduce. In attempting to reproduce the issue with my application, sometimes it is the player that hits the 1000 receiver limit and the application does not crash in that case. There is just a player error.

Since both the the player and the DownloadHelper register an AudioCapabilitiesReceiver, it is a 50/50 chance which will run into the 1000 receiver limit first. But, it is the DownloadHelper that is registering receivers and not releasing them. The player always unregisters the listeners that in registers. The dumpstate is too big to attach here or email. I have attached the stack trace for the case where the application crashes.

stack_trace.txt

For you reproduction attempts, you definitely need to prepare the media. Also, the media selected has to be something that results in DownloadHelper.onMediaPrepared() calling runTrackSelection. That is when the AudioCapabilitiesReceiver is registered.

Also, the media selected has to be something that results in DownloadHelper.onMediaPrepared() calling runTrackSelection.

Thanks, this was the clue I was missing. Switching to using the Clear DASH -> HD (MP4, H264) sample, and then fixing a bug in my second repro case above (adding missing return at the bottom of the if) I'm able to reproduce.

@retiredreplicant

@retiredreplicant @icbaker
I was facing the same issue and the following pull request changes worked for me.

#1230

Ensure DownloadHelper doesn't leak unreleased Renderer instances

This is fixed by the commit referenced above.

Ensure DownloadHelper doesn't leak unreleased Renderer instances

When can we expect this to be included in final release?

The fix will be included in the 1.4.0 release.

Unfortunately c5e894e was rolled back in c2356e3 due to causing crashes when integrated with a google app which calls DownloadHelper.forMediaItem from a non-looper thread. Re-opening this issue while we look into resolving this and rolling the fix back forward.

@icbaker

@icbaker Since above mentioned commit is rolled back, can we consider this PR now? This worked in our case

#1230

The fix has been resubmitted with a fix & related tests: fad3257

Navigation Menu

Repository navigation

Application using the library will crash if 1000 DownloadHelper instances are created with a non-null RenderersFactory #1224

Version

More version details

Devices that reproduce the issue

Devices that do not reproduce the issue

Reproducible in the demo app?

Reproduction steps

Expected result

Actual result

Media

Bug Report

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (startDownloadDialogHelper != null) {
	startDownloadDialogHelper.release();
	}
	startDownloadDialogHelper =
	new StartDownloadDialogHelper(
	fragmentManager,
	DownloadHelper.forMediaItem(context, mediaItem, renderersFactory, dataSourceFactory),
	mediaItem);
	}

Application using the library will crash if 1000 DownloadHelper instances are created with a non-null RenderersFactory #1224

Description

Version

More version details

Devices that reproduce the issue

Devices that do not reproduce the issue

Reproducible in the demo app?

Reproduction steps

Expected result

Actual result

Media

Bug Report

Activity

icbaker commented on Mar 27, 2024

retiredreplicant commented on Mar 27, 2024

retiredreplicant commented on Mar 27, 2024

icbaker commented on Mar 27, 2024

sahilgarg90 commented on Mar 28, 2024

icbaker commented on Mar 28, 2024

sahilpocketfm commented on Mar 29, 2024

icbaker commented on Apr 2, 2024

icbaker commented on Apr 8, 2024

sahilpocketfm commented on Apr 8, 2024

icbaker commented on Apr 8, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions