androidx / media

Jetpack Media3 support libraries for media use cases, including ExoPlayer, an extensible media player for Android
https://developer.android.com/media/media3
Apache License 2.0
1.75k stars 418 forks source link

NetworkTypeObserver makes binding calls on mainthread by default #1550

Open Tolriq opened 4 months ago

Tolriq commented 4 months ago

Version

Media3 main branch

More version details

No response

Devices that reproduce the issue

NetworkTypeObserver register it's receiver to get onReceive calls on mainthread this leads to binding calls to system services on mainthread and can cause ANR and lock main thread.

This should be registered in a background handled to avoid this.

There's quite a few other places where this happens,

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Yes

Reproduction steps

check the receiver calls thread.

Expected result

No binding calls in mainthread.

Actual result

Binding calls in mainthread.

Media

N/A

Bug Report

marcbaechinger commented 4 months ago

Have you seen such ANRs that you can post here?

I'm asking because I know of framework code that do binder calls on the main thread. That code was written by people who I'd say understand more than me around this.

While I'm not saying you are wrong, I'm suprised to not see this documented in Context.regeisterReceiver and finding other Android framework code that is doing binder calls. In theory, any method call can block longer than a caller wanted to. So I'd like to understand how a binder call to a healthy system binder is more risky than other calls.

Can you educate me/us before I reassign? :)

Tolriq commented 4 months ago

The problem is not the register, the problem is that you call register without passing an handler so the onReceive is called on main thread. (See below for a stackTrace)

Then in the onReceive you call for example getNetworkTypeFromConnectivityManager that access context.getSystemService(Context.CONNECTIVITY_SERVICE) and call for example getActiveNetworkInfo this call is a service call via IPC and binding and so can block.

This is well documented and should be avoided, devices can have too much binding at some point, be slow or on limits for those calls.

In this case there's no need to do those calls in mainthread so it's easy to avoid potential issue. (See also for example : https://github.com/google/ExoPlayer/issues/11138 for the same kind of issues, those can be workaround by init ExoPlayer in the background, for this one this is not possible to workaround client side.).

You should either call the register in NetworkTypeObserver constructor with a background handler like

    HandlerThread handlerThread = new HandlerThread("BackgroundReceiverThread" , android.os.Process.THREAD_PRIORITY_BACKGROUND);
    handlerThread.start();
    Looper looper = handlerThread.getLooper();
    Handler backgroundHandler = new Handler(looper);
    ...
  context.registerReceiver(new Receiver(), filter, null, backgroundHandler);

(Be sure to update updateNetworkType to use mainHandler.post(() -> { to call the listener.

Or you can a start background task from inside the onReceive

ANR trace:

          main (native):tid=1 systid=11543 
#00 pc 0x99000 libc.so (syscall + 32) (BuildId: d1a98b526f2f94260a53c3055979a4f6)
#01 pc 0x23247c libart.so (art::ConditionVariable::WaitHoldingLocks + 140) (BuildId: ddcc440d4609d2099db9d20895487a78)
#02 pc 0x45b218 libart.so (artJniMethodEnd + 336) (BuildId: ddcc440d4609d2099db9d20895487a78)
#03 pc 0x5bf0fc libart.so (art_jni_method_end + 12) (BuildId: ddcc440d4609d2099db9d20895487a78)
       at android.os.BinderProxy.transactNative(Native method)
       at android.os.BinderProxy.transact(BinderProxy.java:685)
       at android.net.IConnectivityManager$Stub$Proxy.getActiveNetworkInfo(IConnectivityManager.java:1769)
       at android.net.ConnectivityManager.getActiveNetworkInfo(ConnectivityManager.java:1387)
       at androidx.media3.common.util.NetworkTypeObserver.getNetworkTypeFromConnectivityManager(NetworkTypeObserver.java:157)
       at androidx.media3.common.util.NetworkTypeObserver.access$100(NetworkTypeObserver.java:49)
       at androidx.media3.common.util.NetworkTypeObserver$Receiver.onReceive(NetworkTypeObserver.java:217)
       at android.app.LoadedApk$ReceiverDispatcher$Args.lambda$getRunnable$0(LoadedApk.java:1943)
       at android.app.LoadedApk$ReceiverDispatcher$Args.$r8$lambda$gDuJqgxY6Zb-ifyeubKeivTLAwk(unavailable)
       at android.app.LoadedApk$ReceiverDispatcher$Args$$ExternalSyntheticLambda0.run(unavailable:2)
       at android.os.Handler.handleCallback(Handler.java:958)
       at android.os.Handler.dispatchMessage(Handler.java:99)
       at android.os.Looper.loopOnce(Looper.java:257)
       at android.os.Looper.loop(Looper.java:368)
       at android.app.ActivityThread.main(ActivityThread.java:8839)
       at java.lang.reflect.Method.invoke(Native method)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:572)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1049)