Caused by: java.lang.IllegalStateException: Activity has been destroyed #850
Comments
|
Something probably needs to be done as it comes up sometimes, but we need a consistent repro to figure out what's going on. Can you consistently repro this? Can you make a mini project that demonstrates? Are you doing anything Glide related in the teardown methods or async task? Or more generally can you share the relevant bits of the code? Can you please read #795 and #803 to see where we stand around onDestroy, yours it not necessarily a dupe, but those two should give you ideas. For what I can decipher from the stack: there was a load started earlier (say, |
|
Thanks 4 your reply @TWiStErRob . This Stack trace was reported by my app's built-in User Behavior Collector and I've noticed it because this crash was frequently happened in these days. Actually I have not reproduce the bug by myself till now but it really looks not like a joke. I think I'll follow your advice to have a try and will provide you with a consistent repro if I need your further help. |
|
Please report back even if you found the problem and fixed it, because it may help others in the future. |
|
I also saw this exception several times. Usually it causes when activity closes and in the same time recyclerView/listView adapter tries to draw views, which uses glide. I fixed this with creating glide wrapper. public static void networkImage(Context context, String imageUrl, ImageView targetView, RequestListener requestListener, Drawable imagePlaceholder) {
if (context instanceof Activity) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1 && ((Activity) context).isDestroyed()) {
return;
}
}
if (context != null) {
Logger.d("Loading network image: %s", imageUrl);
DrawableRequestBuilder requestBuilder = Glide.with(context)
.load(imageUrl)
.diskCacheStrategy(DiskCacheStrategy.ALL);
if (requestListener != null) {
requestBuilder.listener(requestListener);
}
if (imagePlaceholder != null) {
requestBuilder.placeholder(imagePlaceholder);
}
requestBuilder.into(targetView);
}
}But then, I realized that if I use context from targetView, this exception doesn't throw public static void networkImage(String imageUrl, ImageView targetView, RequestListener requestListener, Drawable imagePlaceholder) {
if (targetView != null) {
Logger.d("Loading network image: %s", imageUrl);
DrawableRequestBuilder requestBuilder = Glide.with(targetView.getContext())
.load(imageUrl)
.diskCacheStrategy(DiskCacheStrategy.ALL);
if (requestListener != null) {
requestBuilder.listener(requestListener);
}
if (imagePlaceholder != null) {
requestBuilder.placeholder(imagePlaceholder);
}
requestBuilder.into(targetView);
}
} |
|
Huh, that It sounds like we need to clear the adapter ( |
|
I'd like to share what I'm trying to do with this problem in these days. In my app, The scene or usage is: I was trying to load a pic in a fragment, let's call it MyMainFragment, the MyMainFragment's attached activity maintained a AsyncTask object where I was trying to add this fragment inside its onPostExecute() method, and the example code was like: private class FireUpTask extends AsyncTask<Void, Void,Void> {
@Override
protected Void doInBackground(Void... params) {
handleIntent();
return null;
}
@Override
protected void onPostExecute(Void aVoid) {
super.onPostExecute(aVoid);
initFragment();
}
}
private void initFragment() {
if (!isDestroyed()) {
FragmentTransaction fragmentTransaction = getFragmentManager().beginTransaction();
fragmentTransaction.add(R.id.main_container, MyMainFragment.newInstance());
fragmentTransaction.commitAllowingStateLoss();
}
}I was trying to execute FireUpTask inside Activity's onCreate() method without checking the activity's current state, and maybe this was the murderer of this bug. I looked up AOSP, and I was wondering if this should be the proper way to update your ui or your fragment on the AsyncTask object. http://androidxref.com/6.0.0_r1/xref/packages/apps/Dialer/src/com/android/dialer/calllog/ClearCallLogDialog.java#74 So maybe I should modify my code like this: private class FireUpTask extends AsyncTask<Void, Void,Void> {
@Override
protected Void doInBackground(Void... params) {
handleIntent();
return null;
}
@Override
protected void onPostExecute(Void aVoid) {
super.onPostExecute(aVoid);
if (isDestroyed() || isFinishing()) {
return;
}
initFragment();
}
}
private void initFragment() {
FragmentTransaction fragmentTransaction = getFragmentManager().beginTransaction();
fragmentTransaction.add(R.id.main_container, MyMainFragment.newInstance());
fragmentTransaction.commitAllowingStateLoss();
}I've pushed this change to my git repository, the result still not been tested but I think this should make sense. |
|
Thanks for the details, that looks reasonable. You could also leverage |
|
@Jayvd Try to review your code, make sure your entrance of using glide is safe. I've got no idea if Glide will fix this in future release. Looks like we can avoid this by ourself. |
|
@Jayvd I think it is just bad practice to swallow everything. There are some cases when you need to figure out a better way of doing things. I hate that my devices live one day on battery and I think this is one example which is a potential for drain. You want to do something that the user doesn't want to and won't ever see, so why bother the CPU/network/I/O usage -> battery drain?
From personal experience, sadly, that's when you're entering the real world of Android, and advanced framework usage. @sjudd what do you think, would it be reasonable to check the condition ourselves? |
|
When the Activity is destroyed, Glide is unable to start a new load (if an application context or fragment context is provided). We can't start a new load because we're unable to obtain a valid Context. We can either silently ignore the request, or we can throw an exception. Typically this error is a developer error that occurs because someone is misusing AsyncTasks (allowing them to finish after an Activity is destroyed) or otherwise attempting to start loads that will never complete. That said, I think we also found a case where it occurs due to weirdness in the framework or support libraries around nested Fragments. There is a work around for the nested Fragment case where there is no developer error where you can simply acquire RequestManager early in OnCreate using Glide.with() and pass it around rather than calling Glide.with() for every new load. I'm still not convinced that we're better off silently not starting a load, given that the common case is a developer error and that a workaround exists. |
|
I'm afraid I have to reopen this issue cuz it was sadly happened again. |
|
@sjudd This error occurs in a high probability while our testers running monkey test, though real users may not encounter this problem, but i still believe that there must be something wrong. |
|
Can you acquire a recording of the monkey to see what's happening? (Logs/screenshots) |
|
I do have the same issue. It happens when I moved from fragment support to "native" Fragment |
|
Hello @TWiStErRob: I want to ask you a question, thanks. @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR1)
RequestManagerFragment getRequestManagerFragment(final android.app.FragmentManager fm) {
RequestManagerFragment current = (RequestManagerFragment) fm.findFragmentByTag(FRAGMENT_TAG); <---- line: 153
if (current == null) {
current = pendingRequestManagerFragments.get(fm);
if (current == null) {
current = new RequestManagerFragment();
pendingRequestManagerFragments.put(fm, current);
fm.beginTransaction().add(current, FRAGMENT_TAG).commitAllowingStateLoss(); <----- (2) line: 159
handler.obtainMessage(ID_REMOVE_FRAGMENT_MANAGER, fm).sendToTarget();
}
}
return current;
}Caused by: java.lang.IllegalStateException: Activity has been destroyed
at android.app.FragmentManagerImpl.enqueueAction(FragmentManager.java:1383)
at android.app.BackStackRecord.commitInternal(BackStackRecord.java:745)
at android.app.BackStackRecord.commitAllowingStateLoss(BackStackRecord.java:725)
at com.bumptech.glide.manager.RequestManagerRetriever.getRequestManagerFragment(SourceFile:159) <--- (2)
at com.bumptech.glide.manager.RequestManagerFragment.onAttach(SourceFile:117) <----- (1)See (1), RequestManagerFragment instance is already exist! At this moment, why still run into line: 159? That means fm.findFragmentByTag(FRAGMENT_TAG); in line: 153 return null? |
|
@lihanguang huh interesting. So you're expecting |
|
I just see that code that glide dev commit after glide version 3.6.1. // after version 3.6.1
private void registerFragmentWithRoot(Activity activity) {
unregisterFragmentWithRoot();
rootRequestManagerFragment = RequestManagerRetriever.get()
.getRequestManagerFragment(activity.getFragmentManager(), null);
if (rootRequestManagerFragment != this) {
rootRequestManagerFragment.addChildRequestManagerFragment(this);
}
}
@Override
public void onAttach(Activity activity) {
super.onAttach(activity);
try {
registerFragmentWithRoot(activity);
} catch (IllegalStateException e) {
// OnAttach can be called after the activity is destroyed, see #497.
if (Log.isLoggable(TAG, Log.WARN)) {
Log.w(TAG, "Unable to register fragment with root", e);
}
}
}Q1Is the patch about #497 and this issue? Additionly, i think @shawnlinboy 's problem is the quote below while run monkeytest.
Can I avoid the quote above like below? Because commitAllowingStateLoss schedules a commit of this transaction. The commit doesnot happen immediately! // RequestManagerRetriever.java
RequestManagerFragment getRequestManagerFragment(
final android.app.FragmentManager fm, android.app.Fragment parentHint) {
RequestManagerFragment current = (RequestManagerFragment) fm.findFragmentByTag(FRAGMENT_TAG);
if (current == null) {
current = pendingRequestManagerFragments.get(fm);
if (current == null) {
current = new RequestManagerFragment();
current.setParentFragmentHint(parentHint);
pendingRequestManagerFragments.put(fm, current);
fm.beginTransaction().add(current, FRAGMENT_TAG).commitAllowingStateLoss();
fm.executePendingTransactions(); // <------- add this line......
handler.obtainMessage(ID_REMOVE_FRAGMENT_MANAGER, fm).sendToTarget();
}
}
return current;
} |
|
That part you quoted is not valid. That was my hypothesis, but it was debunked when I looked at the code. Do you have a consistent repro so you can test the behaviour of this modification? |
|
You can't use executePendingTransactions, see #117. |
|
I also got the similar exception, I think exception will happen if happens any leak in the view (activity, fragment or dialog etc). So need to check that view is not destroyed before to load image from a URL. |
|
I have also got this issue when used solution from #1337 for clearing views in fragment's private RequestManager imageLoader;
@Override
public void onDestroyView() {
super.onDestroyView();
getImageLoader().onDestroy();
}
protected RequestManager getImageLoader() {
if (imageLoader == null) {
imageLoader = Glide.with(this);
}
return imageLoader;
}If
So my solution was to avoid creating of @Override
public void onDestroyView() {
super.onDestroyView();
if (imageLoader != null) {
imageLoader.onDestroy();
}
}I suppose that |
|
|
|
@jt-gilkeson hmm, that's an interesting issue. I think that shouldn't happen if you use the fragment's |
|
@TWiStErRob That may be my problem, I was using getContext() in the fragment, not "this" (I missed the distinction). Just verified that using "this" does isolate the destroy - my error. Thanks for the follow up and sorry for the confusion. |
|
Maybe someone will find this useful: |
|
@jt-gilkeson Yep, if you destroy the activity manager everything loaded with the activity manager is cleared: http://stackoverflow.com/a/32887693 Remember that manually calling |
|
@TWiStErRob I imagine having to do any of this stuff to not crash your app, is not how the API was intended to be designed :) |
|
True that, but #850 (comment). Also this is likely not fixed yet, because there's no consistent repro that is not coming from a weird Glide usage (starting a load after destroy). |
|
Gotcha, so maybe we don't need any of this cleanup stuff if we would have correctly passed in the fragment's this pointer in the first place? It looks like the original implementation we had was passing in getActivity() (which, when we saw the crash show up, we mistakenly changed to getContext() from the fragment - thinking it would use the fragment). Sorry for all the confusion - when we saw the crash we looked for issues containing the crash and ended up down this path - which now sounds like a red herring, when the real issue for us was not using the right with() param. Thanks for the help - we have multiple people copying/pasting and things get confused easily. |
|
OK I've got a reproducible instance of the original issue, which may help track down the issue: We have a tablet layout which in landscape has a fragment with a master / detail layout. The master fragment (retained instance) has on the left hand side a list of items, and a frame on the right hand side where an embedded fragment gets loaded. If you rotate to portrait, only the master view is shown and clicking on the list selection launches an individual activity. The landscape detail view can contain a UserReviewsFragment (nested) fragment (if selected in the list) where we are calling:
In the master fragment, we switch the fragment in the detail view via: Where frag may be UserReviewsFragment.newInstance(); or another fragment created via newInstance(); If we rotate to portrait (which does not have the detail fragment), then rotate back to landscape, when we try to select the UserReviewsFragment (which has the glide call in it), it crashes with the following: The key seems to be rotating, then initializing the nested fragment - if you don't rotate, the fragment uses Glide with no problem. So repro steps: Open the master/detail fragment (UserReviewsFragment doesn't ever have to be created), Rotate to portrait, Rotate back to landscape, click on item to instantiate and show the nested UserReviewsFragment - crash. PS: The onDestroyView workaround does not work for this, it crashes when a new fragment gets created and mRequestManager.load() gets called. Note: |
|
I've created a new item for this with a Demo app, since my repro steps are very specific (and I've simplified quite a bit from above) |
|
Duplicate of #1592 |
and others
My Glide version is 3.6.1 and here I got a IllegalStateException.
Should Glide handle this problem internally?
The text was updated successfully, but these errors were encountered: