pwittchen / ReactiveNetwork

Android library listening network connection state and Internet connectivity with RxJava Observables
http://pwittchen.github.io/ReactiveNetwork/docs/RxJava2.x/
Apache License 2.0
2.53k stars 276 forks source link

Fatal Exception: MarshmallowNetworkObservingStrategy.java line 80 #412

Open adaonder opened 4 years ago

adaonder commented 4 years ago

BUG

  1. MarshmallowNetworkObservingStrategy.java line 80
  2. com.github.pwittchen.reactivenetwork.library.rx2.network.observing.strategy.MarshmallowNetworkObservingStrategy.observeNetworkConnectivity

LOG

Fatal Exception: java.lang.RuntimeException: Unable to resume activity {com.trt.trt/com.trt.trt.activity.scanner.BeaconScannerActivity}: android.net.ConnectivityManager$TooManyRequestsException
       at android.app.ActivityThread.performResumeActivity(ActivityThread.java:4430)
       at android.app.ActivityThread.handleResumeActivity(ActivityThread.java:4470)
       at android.app.servertransaction.ResumeActivityItem.execute(ResumeActivityItem.java:51)
       at android.app.servertransaction.TransactionExecutor.executeLifecycleState(TransactionExecutor.java:145)
       at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:70)
       at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2199)
       at android.os.Handler.dispatchMessage(Handler.java:112)
       at android.os.Looper.loop(Looper.java:216)
       at android.app.ActivityThread.main(ActivityThread.java:7625)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:524)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:987)

Smartphone (please complete the following information):

pwittchen commented 4 years ago

Thanks for reporting that. When did you get this error? What are the steps to reproduce it? It looks like connectivity manager inside Android SDK received too many requests. I haven't encountered that error before.

adaonder commented 4 years ago

I updated the error report. It happens at a random time. There is no such problem on my device. There is such a problem and I sent it for you to see. There is no serious problem right now. I noticed it through Firebase Crashlytics.

pwittchen commented 4 years ago

Ok, I'll have a look at this.

adaonder commented 4 years ago

New Log;

Caused by android.net.ConnectivityManager$TooManyRequestsException at android.net.ConnectivityManager.convertServiceException(ConnectivityManager.java:3227) at android.net.ConnectivityManager.sendRequestForNetwork(ConnectivityManager.java:3446) at android.net.ConnectivityManager.registerNetworkCallback(ConnectivityManager.java:3760) at android.net.ConnectivityManager.registerNetworkCallback(ConnectivityManager.java:3742) at com.github.pwittchen.reactivenetwork.library.rx2.network.observing.strategy.MarshmallowNetworkObservingStrategy.observeNetworkConnectivity(MarshmallowNetworkObservingStrategy.java:80) at com.github.pwittchen.reactivenetwork.library.rx2.ReactiveNetwork.observeNetworkConnectivity(ReactiveNetwork.java:92) at com.github.pwittchen.reactivenetwork.library.rx2.ReactiveNetwork.observeNetworkConnectivity(ReactiveNetwork.java:73) at com.trt.common.activity.AbstractActivity.onResume(AbstractActivity.java:99) at com.trt.trt.activity.DashboardActivity.onResume(DashboardActivity.java:243) at android.app.Instrumentation.callActivityOnResume(Instrumentation.java:1416) at android.app.Activity.performResume(Activity.java:7585) at android.app.ActivityThread.performResumeActivity(ActivityThread.java:4018) at android.app.ActivityThread.handleResumeActivity(ActivityThread.java:4058) at android.app.servertransaction.ResumeActivityItem.execute(ResumeActivityItem.java:51) at android.app.servertransaction.TransactionExecutor.executeLifecycleState(TransactionExecutor.java:145) at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:70) at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1960) at android.os.Handler.dispatchMessage(Handler.java:106) at android.os.Looper.loop(Looper.java:214) at android.app.ActivityThread.main(ActivityThread.java:7094) at java.lang.reflect.Method.invoke(Method.java) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:494) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:975)

0xGuybrush commented 4 years ago

I was able to reproduce this —

  1. I had a BaseActivity class that multiple activities extended from.
  2. In its onResume I was creating an instance of ReactiveNetwork.observeNetworkConnectivity()
  3. In some activities this was being subscribed to & in those cases the Subscription was disposed in onPause().
  4. In other activities thoguh it wasn't being used, so there was no unsubscription.

I thought that this would be OK, as I hadn't subscribed to anything, but I think what happened there though is that in the call to MarshmallowNetworkObservingStrategy.observeNetworkConnectivity, there is a call to registerNetworkCallback directly which is only unregistered in doOnCancel.

In normal testing I wasn't hitting any issues with this, but as per this post — it looks like the upper limit on NetworkCallbacks is 100.

If I toggle between two activities that instantiate (but don't use) ReactiveNetwork.observeNetworkConnectivity over 100 times, I'll hit the above exception.

I think now that I know why I'm hitting it it should be straightforward to avoid but perhaps moving manager.registerNetworkCallback(request, networkCallback); into doOnSubscribe could avoid this?

return connectivitySubject
        .toFlowable(BackpressureStrategy.LATEST)
       .doOnSubscribe(__ -> manager.registerNetworkCallback(request, networkCallback))
        .doOnCancel(new Action() {
            @Override public void run() {
                tryToUnregisterCallback(manager);
                tryToUnregisterReceiver(context);
            }
        })
       // …

EDIT: Happy to provide some example code to help reproduce if that's useful, (or a PR if the above suggestion makes sense)?

pwittchen commented 4 years ago

Hi @another-dave. Thanks for your comment. I didn't know about this limitation. If applying this slight change will prevent these errors or will reduce them, I think it's worth introducing it into the code-base.

denys-meloshyn commented 3 years ago

@pwittchen do you have any updates about this issue?

adarsh-kh4u commented 3 years ago

I'm facing same issue