0.3.5: Observable not being cached and fragments / activities being leaked
Closed this issue · 8 comments
Hi there!
First, thank you for making this library, it looks like it will solve many issues with mobile applications when it comes to doing asynchronous work and sudden configuration changes.
However, I've run into an issue and I am not entirely sure what I am doing incorrectly as I believe I followed the sample closely
Introduction to app
Simple app that uses Retrofit to fetch a list of users. Once fetched, these users are displayed on a RecyclerView that is apart of a fragment
Summary of problems
My onStart method within the fragment looks like this
@Override
public void onStart() {
super.onStart();
if(mUsers == null && !mGroupLifecycleManager.hasObservable(TAG))
loadUsers();
}
So it checks if we do not have users already and the manager does not have the observable
On a configuration change, the hasObservable call always returns false even though I am forwarding the appropriate life cycle calls
Once the task is done, it calls the following method in onNext
private void displayUsers(){
showContent();
UserAdapter userAdapter = new UserAdapter(mUsers);
mRecyclerView.setAdapter(userAdapter);
String randomString = getString(R.string.app_name);
}
When I rotate the device whilst the task is ongoing, I will eventually I get the following error in my logcat
11-08 23:06:41.416 763-763/com.ersen.rxgroupissue I/UsersFragment: onError java.lang.IllegalStateException: Fragment UsersFragment{2fcd9c4} not attached to Activity
at android.support.v4.app.Fragment.getResources(Fragment.java:648)
at android.support.v4.app.Fragment.getString(Fragment.java:670)
at com.ersen.rxgroupissue.fragments.UsersFragment.displayUsers(UsersFragment.java:107)
at com.ersen.rxgroupissue.fragments.UsersFragment.access$200(UsersFragment.java:37)
at com.ersen.rxgroupissue.fragments.UsersFragment$1.onNext(UsersFragment.java:131)
at com.ersen.rxgroupissue.fragments.UsersFragment$1.onNext(UsersFragment.java:110)
at rx.internal.util.ObserverSubscriber.onNext(ObserverSubscriber.java:34)
at rx.observers.SafeSubscriber.onNext(SafeSubscriber.java:134)
at rx.internal.operators.OperatorSubscribeOn$1$1.onNext(OperatorSubscribeOn.java:53)
at rx.internal.operators.OperatorObserveOn$ObserveOnSubscriber.call(OperatorObserveOn.java:227)
at rx.android.schedulers.LooperScheduler$ScheduledAction.run(LooperScheduler.java:107)
at android.os.Handler.handleCallback(Handler.java:739)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:148)
at android.app.ActivityThread.main(ActivityThread.java:5417)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)
I decided to look at a heap snapshot and I encountered that I had many MainAcitivity and UserFragment instances left in memory after configuration change. This is probably the cause of the above exception, somehow it called the method on a dead leftover instance of the fragment.
Here is a link to the project which replicates the issue I am having. I would be very grateful if you could look over this.
https://www.dropbox.com/s/ooipzvxqxeobipi/RxGroupIssue.rar?dl=0
Note that I forward the lifecycles to GroupLifecycleManager in the BaseFragment class
Any more information needed, let me know
Cheers!
Are you calling GroupLifecycleManager from all your lifecycle methods?
Check out MainActivity in the sample app, you should be doing something similar
@felipecsl
Hi, many thanks the reply. I will give a try this weekend and i'll report back my findings.
I am only forwarding the fragment ones but not the activity
that should be fine too
@felipecsl
Hi, I double checked the lifecycle calls, I am calling them all for the fragment and activity. I am still experiencing the mentioned issues
BaseActivity
@Override
public void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Log.i(TAG,"onCreate");
mGroupLifecycleManager = GroupLifecycleManager.onCreate(
RxGroupApplication.getInstance().getObservableManager(),
savedInstanceState, this);
}
@Override
public void onPause() {
super.onPause();
Log.i(TAG,"onPause");
mGroupLifecycleManager.onPause();
}
@Override
public void onResume() {
super.onResume();
Log.i(TAG,"onResume");
mGroupLifecycleManager.onResume();
}
@Override
public void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
Log.i(TAG,"onSaveInstanceState");
mGroupLifecycleManager.onSaveInstanceState(outState);
}
@Override
public void onDestroy() {
super.onDestroy();
Log.i(TAG,"onDestroy");
mGroupLifecycleManager.onDestroy(this);
}
BaseFragment
@Override
public void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Log.i(TAG,"onCreate");
mGroupLifecycleManager = GroupLifecycleManager.onCreate(
RxGroupApplication.getInstance().getObservableManager(),
savedInstanceState, this);
}
@Override
public void onPause() {
super.onPause();
Log.i(TAG,"onPause");
mGroupLifecycleManager.onPause();
}
@Override
public void onResume() {
super.onResume();
Log.i(TAG,"onResume");
mGroupLifecycleManager.onResume();
}
@Override
public void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
Log.i(TAG,"onSaveInstanceState");
mGroupLifecycleManager.onSaveInstanceState(outState);
}
@Override
public void onDestroy() {
super.onDestroy();
Log.i(TAG,"onDestroy");
mGroupLifecycleManager.onDestroy(getActivity());
}
The link to the app has been updated too
OK @erseno I spent some time debugging your project to see what's going on, here's what I found out so far:
You're doing:
userObservable
.compose(mGroupLifecycleManager.<ArrayList<User>>transform(TAG))
.delay(5, TimeUnit.SECONDS)If you just invert the order of compose and delay, so that it looks like it's shown below, then it works:
userObservable
.delay(5, TimeUnit.SECONDS)
.compose(mGroupLifecycleManager.<ArrayList<User>>transform(TAG))As a side effect, you'll have to manually move your code in onNext into the main thread since it will be called from a background thread.
This is just a workaround to get you unblocked, in the meantime I'll look into why this is happening.
What I've seen so far is that ObservableGroup#add is causing the onTerminate callback to be invoked immediately after managedObservable#unlock is called.
I'll keep digging on to why this is happening and will update this issue when I have a fix.
Thanks for your great bug report and providing the test project.
OK so this is not a bug and working as intended. Remember that RxJava operators can't see the future, so they can't see anything that's applied after them. In this case specifically, the delay call.
What's happening is that the Retrofit response is returning very fast (under 200ms average on my machine) and you're applying the RxGroups transformer on it. By the time that the response is received from Retrofit, RxGroups immediately removes that Observable from the list of "in flight" requests since it's already seen a terminal event (onCompleted). So, when you rotate, delay() is grabbing that event and holding onto it for 5 seconds, thus finally delivering it after the Fragment has been already destroyed, causing the Exception.
When you move the delay call above compose, now RxGroups is aware of the entire stream and is able to correctly manage rotation since it will prevent delay() from delivering the event after onDestroy().
So TL;DR: not a bug 😄
Hi thanks for the comments and apologies for my late reply.
I've removed the delay call and made the emulator simulate a slow network connection (GPRS) so that the call takes a while. I am still encountering the same mentioned issues.
I've updated my project again.
Below the difference.
Observable<ArrayList<User>> userObservable = RetrofitManager.INSTANCE.getApiService().getUsers();
Observable<ArrayList<User>> userObservableTwo = RetrofitManager.INSTANCE.getApiService().getUsers();
Observable<ArrayList<User>> userObservableThree = RetrofitManager.INSTANCE.getApiService().getUsers();
Observable<ArrayList<User>> userObservableFour = RetrofitManager.INSTANCE.getApiService().getUsers();
Observable<ArrayList<User>> userObservableFive = RetrofitManager.INSTANCE.getApiService().getUsers();
// Observable.zip(userObservable, userObservableTwo, userObservableThree,userObservableFour,userObservableFive, (users, users2, users3,users4,users5) -> {
// ArrayList<User> usersCombined = new ArrayList<>();
// usersCombined.addAll(users);
// usersCombined.addAll(users2);
// usersCombined.addAll(users3);
// usersCombined.addAll(users4);
// usersCombined.addAll(users5);
// return usersCombined;
// })
// .compose(mObservableGroup.<ArrayList<User>>transform(TAG))
// .observeOn(AndroidSchedulers.mainThread())
// .subscribeOn(Schedulers.newThread())
// .subscribe(mObserver);
userObservable
.compose(mObservableGroup.<ArrayList<User>>transform(TAG))
.observeOn(AndroidSchedulers.mainThread())
.subscribeOn(Schedulers.newThread())
.subscribe(mObserver);
The commented stuff was just to simulate a longer network request before I realised that it is possible for the emulator to simulate a slower internet.
Cheers!