Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NetworkTypeObserver makes binding calls on mainthread by default #1550

Open
1 task
Tolriq opened this issue Jul 17, 2024 · 2 comments
Open
1 task

NetworkTypeObserver makes binding calls on mainthread by default #1550

Tolriq opened this issue Jul 17, 2024 · 2 comments
Assignees
Labels

Comments

@Tolriq
Copy link
Contributor

Tolriq commented Jul 17, 2024

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

  • You will email the zip file produced by adb bugreport to [email protected] after filing this issue.
@marcbaechinger
Copy link
Contributor

marcbaechinger commented Jul 17, 2024

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
Copy link
Contributor Author

Tolriq commented Jul 17, 2024

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 : google/ExoPlayer#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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants