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

"NetworkCallback was already unregistered" if re-using Observable #428

Closed 0xGuybrush closed 4 years ago

0xGuybrush commented 4 years ago

Describe the bug

Subscribing to a single "internet connectivity check" from two different disposables causing an IllegalArgumentException when disposing obserable chains on pause.

To Reproduce Steps to reproduce the behavior:

  1. Create an Observable for Internet connectivity, e.g.
final Observable<Boolean> hasInternet = ReactiveNetwork.observeNetworkConnectivity(getApplicationContext())
                                                       .switchMapSingle(connectivity -> ReactiveNetwork.checkInternetConnectivity());
  1. Subscribe to the observable from two different disposables:
CompositeDisposable compositeDisposable = new CompositeDisposable();

// One
compositeDisposable.add(hasInternet.subscribeOn(Schedulers.io()).observeOn(AndroidSchedulers.mainThread()).subscribe(foo, bar));

// Two
compositeDisposable.add(hasInternet.subscribeOn(Schedulers.io()).observeOn(AndroidSchedulers.mainThread()).subscribe(baz, qux));
  1. Configure CompositeDisposable to dispose onPause
@Override
public void onPause() {
    super.onPause();
    compositeDisposable.dispose();
}
  1. Boot app and trigger onPause (activity change or hide app)

  2. Results in error:

2020-04-07 14:47:58.164 20834-20834/ie.davidmoloney.reactivenetworktest E/ReactiveNetwork: could not unregister network callback
    java.lang.IllegalArgumentException: NetworkCallback was already unregistered
        at com.android.internal.util.Preconditions.checkArgument(Preconditions.java:47)
        at android.net.ConnectivityManager.unregisterNetworkCallback(ConnectivityManager.java:3496)
        at com.github.pwittchen.reactivenetwork.library.rx2.network.observing.strategy.MarshmallowNetworkObservingStrategy.tryToUnregisterCallback(MarshmallowNetworkObservingStrategy.java:140)
        at com.github.pwittchen.reactivenetwork.library.rx2.network.observing.strategy.MarshmallowNetworkObservingStrategy$3.run(MarshmallowNetworkObservingStrategy.java:84)
        at io.reactivex.internal.operators.flowable.FlowableDoOnLifecycle$SubscriptionLambdaSubscriber.cancel(FlowableDoOnLifecycle.java:115)
        at io.reactivex.internal.subscribers.BasicFuseableSubscriber.cancel(BasicFuseableSubscriber.java:158)
        at io.reactivex.internal.operators.flowable.FlowableFlatMap$MergeSubscriber.cancel(FlowableFlatMap.java:358)
        at io.reactivex.internal.subscriptions.SubscriptionArbiter.drainLoop(SubscriptionArbiter.java:221)
        at io.reactivex.internal.subscriptions.SubscriptionArbiter.drain(SubscriptionArbiter.java:190)
        at io.reactivex.internal.subscriptions.SubscriptionArbiter.cancel(SubscriptionArbiter.java:182)
        at io.reactivex.internal.subscribers.BasicFuseableSubscriber.cancel(BasicFuseableSubscriber.java:158)
        at io.reactivex.internal.operators.observable.ObservableFromPublisher$PublisherSubscriber.dispose(ObservableFromPublisher.java:70)
        at io.reactivex.internal.operators.mixed.ObservableSwitchMapSingle$SwitchMapSingleMainObserver.dispose(ObservableSwitchMapSingle.java:165)
        at io.reactivex.internal.disposables.DisposableHelper.dispose(DisposableHelper.java:124)
        at io.reactivex.internal.operators.observable.ObservableSubscribeOn$SubscribeOnObserver.dispose(ObservableSubscribeOn.java:73)
        at io.reactivex.internal.operators.observable.ObservableObserveOn$ObserveOnObserver.dispose(ObservableObserveOn.java:146)
        at io.reactivex.internal.disposables.DisposableHelper.dispose(DisposableHelper.java:124)
        at io.reactivex.internal.observers.LambdaObserver.dispose(LambdaObserver.java:102)
        at io.reactivex.disposables.CompositeDisposable.dispose(CompositeDisposable.java:240)
        at io.reactivex.disposables.CompositeDisposable.dispose(CompositeDisposable.java:82)
        at ie.davidmoloney.reactivenetworktest.BaseActivity.onPause(BaseActivity.java:79)
        at android.app.Activity.performPause(Activity.java:7337)
        at android.app.Instrumentation.callActivityOnPause(Instrumentation.java:1487)
        at android.app.ActivityThread.performPauseActivityIfNeeded(ActivityThread.java:4241)
        at android.app.ActivityThread.performPauseActivity(ActivityThread.java:4206)
        at android.app.ActivityThread.handlePauseActivity(ActivityThread.java:4158)
        at android.app.servertransaction.PauseActivityItem.execute(PauseActivityItem.java:45)
        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:1977)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6923)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:870)
2020-04-07 14:47:58.170 20834-20834/ie.davidmoloney.reactivenetworktest E/ReactiveNetwork: could not unregister receiver
    java.lang.IllegalArgumentException: Receiver not registered: com.github.pwittchen.reactivenetwork.library.rx2.network.observing.strategy.MarshmallowNetworkObservingStrategy$4@fafb087
        at android.app.LoadedApk.forgetReceiverDispatcher(LoadedApk.java:1280)
        at android.app.ContextImpl.unregisterReceiver(ContextImpl.java:1519)
        at android.content.ContextWrapper.unregisterReceiver(ContextWrapper.java:659)
        at com.github.pwittchen.reactivenetwork.library.rx2.network.observing.strategy.MarshmallowNetworkObservingStrategy.tryToUnregisterReceiver(MarshmallowNetworkObservingStrategy.java:148)
        at com.github.pwittchen.reactivenetwork.library.rx2.network.observing.strategy.MarshmallowNetworkObservingStrategy$3.run(MarshmallowNetworkObservingStrategy.java:85)
        at io.reactivex.internal.operators.flowable.FlowableDoOnLifecycle$SubscriptionLambdaSubscriber.cancel(FlowableDoOnLifecycle.java:115)
        at io.reactivex.internal.subscribers.BasicFuseableSubscriber.cancel(BasicFuseableSubscriber.java:158)
        at io.reactivex.internal.operators.flowable.FlowableFlatMap$MergeSubscriber.cancel(FlowableFlatMap.java:358)
        at io.reactivex.internal.subscriptions.SubscriptionArbiter.drainLoop(SubscriptionArbiter.java:221)
        at io.reactivex.internal.subscriptions.SubscriptionArbiter.drain(SubscriptionArbiter.java:190)
        at io.reactivex.internal.subscriptions.SubscriptionArbiter.cancel(SubscriptionArbiter.java:182)
        at io.reactivex.internal.subscribers.BasicFuseableSubscriber.cancel(BasicFuseableSubscriber.java:158)
        at io.reactivex.internal.operators.observable.ObservableFromPublisher$PublisherSubscriber.dispose(ObservableFromPublisher.java:70)
        at io.reactivex.internal.operators.mixed.ObservableSwitchMapSingle$SwitchMapSingleMainObserver.dispose(ObservableSwitchMapSingle.java:165)
        at io.reactivex.internal.disposables.DisposableHelper.dispose(DisposableHelper.java:124)
        at io.reactivex.internal.operators.observable.ObservableSubscribeOn$SubscribeOnObserver.dispose(ObservableSubscribeOn.java:73)
        at io.reactivex.internal.operators.observable.ObservableObserveOn$ObserveOnObserver.dispose(ObservableObserveOn.java:146)
        at io.reactivex.internal.disposables.DisposableHelper.dispose(DisposableHelper.java:124)
        at io.reactivex.internal.observers.LambdaObserver.dispose(LambdaObserver.java:102)
        at io.reactivex.disposables.CompositeDisposable.dispose(CompositeDisposable.java:240)
        at io.reactivex.disposables.CompositeDisposable.dispose(CompositeDisposable.java:82)
        at ie.davidmoloney.reactivenetworktest.BaseActivity.onPause(BaseActivity.java:79)
        at android.app.Activity.performPause(Activity.java:7337)
        at android.app.Instrumentation.callActivityOnPause(Instrumentation.java:1487)
        at android.app.ActivityThread.performPauseActivityIfNeeded(ActivityThread.java:4241)
        at android.app.ActivityThread.performPauseActivity(ActivityThread.java:4206)
        at android.app.ActivityThread.handlePauseActivity(ActivityThread.java:4158)
        at android.app.servertransaction.PauseActivityItem.execute(PauseActivityItem.java:45)
        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:1977)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6923)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:870)

Expected behavior Expected additional subscribers to observable to be disposed of without issue. (New to RxJava though, so may be my misunderstanding!)

Additional context Tried to create a complete example here — another-dave/reactive-network-test

pwittchen commented 4 years ago

Hi,

Thanks for reporting issue. I suppose this problem may be related to the fact that there is more than one Disposable per from the ReactiveNetwork single Activity and the same objects could reused. Due to this fact, when NetworkCallback is unregistered once, then another one cannot be unregistered because of the same object. I'm not sure about that, but that's my guess.

Regards, Piotr

0xGuybrush commented 4 years ago

Hi Piotr,

thanks for your reply & getting back so quickly.

I have multiple pieces of functionality that depend on network state in some of my activities, so thought the cleanest way was to reuse, but I'll see if I can refactor it though to use a single disposable instead.

Best Dave

0xGuybrush commented 4 years ago

Just to note, if anyone else comes across this, that I now think the correct way of reusing this in RxJava is to turn it into a ConnectableObservable to allow multiple observables to chain to it and will only dispose when the final downstream Observable disconnects.

I've used .share() here to handle this and seems to be working fine to turn it into a "hot" observable. Closing ticket, as it's due to my knowledge gap rather than an issue here 🙂

pwittchen commented 4 years ago

Great! I'm glad you solved this issue. :)