Thanks very much for the detailed report!

What does com.myapp.myapp$24 do?
What triggers it to be scheduled for later execution?

It sounds like what you're getting at is that I can fix the issue on my end instead of making a change in the library. However I do not believe this is the best solution since people use a simplified library like Glide because they don't have to worry about corner cases like this. In my opinion Glide should handle this directly. Obviously I'm no expert on Glide's code so I could be off base here.

To answer your question a network call comes in which then posts to run the code on the UI thread. Since I am in a view there I can do:

onAttachedToWindow
    isAttached = true;

onDetachedFromWindow
    isAttached = false;

// in my method i can call
if (isAttached) {
    //Glide call
}

What do you guys think?

You're right. I kind of agree with you, it would be simpler if Glide ignored this, but at the same time I don't, here's why. I think in general it's just good to know what's going on in your code. Glide ignoring this would hide a potential bug in your code, that you would consider a feature, but at the same time for someone else it may be a good thing that Glide brings it up. The problem is that there isn't really a good mid-way between log and a crash. Most people ignore logging, especially because Glide doesn't log much unless you turn it on explciitly. On the other hand if your app crashes, you're forced to find out why and fix the bug. Even if you could get away with not doing so.

I think in general if you're going from back->UI thread it's a good thing to check if the UI is still alive. Why would you spend any time updating something that doesn't exist, or not visible and never will be visible. Especially taking away from the precious render time and battery life. In this simple case you may not be doing much, but it's really easy to think of similar use cases which are resource heavy. So regarding your implementation idea, I think you can just ignore the whole Runnable ($24) if your activity.isFinishing(); or even better (together with isFinishing) try to cancel the network request if the activity dies.

By the way, to give a little background why that check is in place: Glide attaches a fragment to the activity/fragment if you're using the corresponding with(...). This fragment is used to subscribe to lifecycle events so requests can be automatically paused/resumed and cancelled. See http://stackoverflow.com/a/32887693/253468. Oh, and for this fragment to be added, as a normal one, the activity has to be in good condition. Follow the Glide.with call chain in code to see more.

Thanks for the detailed reply and spending time to look into this, I really appreciate it. It's amazing when devs support their own libraries with this level of support.

Glide is really powerful because, like you are saying, it can respect the lifecycle of activities and fragment. An activity being destroyed is a lifecycle event. Therefore I would argue it should respect and properly handle when your activity is destroyed.

I get where you are coming from but if Glide were to silently fail here the only additional overhead (in my app) would be a single post request which is negligible. This is a very small corner case in my code (13 crashes in 15000 sessions), I wouldn't have found it without crashlytics. Realistically adding ~6 lines of code (the activity destroyed check) isn't a big deal to work around it but I felt that Glide would be better as a library if it handled this situation.

It's amazing when devs support their own libraries with this level of support.

I'm not even a full dev on Glide, I'm just having fun here on Glide issues having my daily dose of brain training. Sam's is the sole owner and main maintainer of Glide. I commit only minor things sometimes.

Considering what I just said, let's see what @sjudd's opinion is on ignoring a load on a destroyed activity with a warn log.

Thanks!

@michaelrhughes
@TWiStErRob

Maybe you can solve this problem instead of using "activity.isFinishing()".

//In Activity
 RequestManager mRequestManager
  public void onCreate(){
    RequestManager mRequestManager= Glide.with(this);
     ....

   Runnable r = new Runnable(){
        mRequestManager.load(glideUrl).into(imageview).
   }

 //TODO:  post Runnable after activity is destoryed.
 }


//In Adapter
RecyclerAdapter(Context context, List<MediaStoreData> data, RequestManager requestManager) {
    this.data = data;
    mRequestManager= requestManager
}

@lihanguang hmm, interesting idea. I'm not sure how it helps though. I mean I don't know whether this will:

1. ignore any loads,
2. go through with them unnecessarily,
3. or just delay the crash for a later time.

These last two (2-3) sound bad, and my guess is that Glide will do 2. when called after destroy.

@TWiStErRob
"Crash when activity is destroyed" means @michaelrhughes call Glide.with(this) after "activity is destoryed", so you can call Glide.with(this) at the certain moment that activity is not destoryed. In my solution above, "the certain moment" is onCreate()

Q1: ignore any loads.
Answer: Donot worry about that. Because i cache the RequestManager instance in onCreate(), then use this instance all the time . You can see the glide demo("com.bumptech.glide.samples.gallery.RecyclerAdapter") .

See RequestManagerFragment.onStart/onStop which calls ActivityFragmentLifecycle which propagates to RequestManager.onStart/onStop.

Q2: go through with them unnecessarily
Answer: "go through with them" means you donnot need to lazy get the instance of RequestManager that attached at the current Context( Activity). So i think "go through with them" in some case is necessarily.
For example: in a adapter ("com.bumptech.glide.samples.gallery.RecyclerAdapter")

//In Adapter
RecyclerAdapter(Context context, List<MediaStoreData> data, RequestManager requestManager) {
    this.data = data;
    mRequestManager= requestManager
}

@Override
    public void onBindViewHolder(final RecyclerView.ViewHolder holder, final int position) {
      mRequestManager.load(glideurl).into(imageview);
}

Q3: or just delay the crash for a later time.
Answer: no delay crash, because see Q1 answer

@lihanguang so it won't do any of what I said, simply because the RequestManager is paused when activity is destroyed, that's a good one :) It also eagerly initializes the magic fragment which prevents this and possible other weirdnesses.

I wonder if @sjudd did this in the samples knowingly. My guess would be that it just made sense to cache it and some APIs need to get it passed: for example the adapter you quoted benefits because it can be used in Activity/Fragment/or even with AppContext by removing the Glide.with dependency from inside adapter.bind and just passing the actual request manager.

@michaelrhughes did you try this?

Passing the RequestManager in rather than constantly calling Glide.with does a number of things:

1. It makes the code testable (you can mock RequestManager)
2. It allows you to start loads with the correct context for Views/Adapters created in Fragments (otherwise you end up with the View Context which points to the Activity, not the Fragment)
3. It avoids the (relatively minimal) overhead of the with call itself

It's not intended to avoid this particular assertion, though it can be useful in limited cases where the assertion can't be avoided due to bugs in how fragments are handled in the support libraries (typically affects deeply nested Fragment hierarchies and/or fragment pagers).

The assertion exists because, aside from the fragment issues I mentioned, it never makes sense to try to start an image load while an Activity is being destroyed. It often indicates that you're doing something else dangerous, like starting an image load in an AsyncTask callback for a task that may finish after onDestroy is called and that isn't cancelled.

More practically, because Glide.with() returns a RequestManager the caller then uses, we'd have to create a special mock or pass along some state that indicates that the load should actually be ignored all the down to the into() call, which would be hard to maintain.

Can using "RequestManager" to solve @michaelrhughes 's problem?

@lihanguang Yes it was a good idea, though he didn't respond yet.

Just as a heads up for people who run into this (perhaps on older versions of the library and aren't going to upgrade right away) - some adapters may be saving the layoutInflater (or use some other "optimization") which after rotation can inflate a viewHolder with the wrong context (which despite being incorrect - works for the most part). If you then call something like imageView.getContext() and use it with Glide you can run into this issue.

I think the way is that we pause request before activity destroyed.

@Override
+    public boolean onKeyDown(int keyCode, KeyEvent event) {
+        if (keyCode == KeyEvent.KEYCODE_BACK) {
+            Glide.with(this).pauseRequests();
+        }
+        return super.onKeyDown(keyCode, event);
+    }
+
+    @Override
+    public void finish() {
+        Glide.with(this).pauseRequests();
+        super.finish();
+    }

The crashes come from calling Glide.with(this) on an invalid context. Doing what you mentioned above will pause requests that are no longer necessary, but it doesn't solve the crash issue.

I am using following util function before loading any image

final Context  context = getContext();
if (isValidContextForGlide(context) {
      // Load image via Glide lib using context
}

 public static boolean isValidContextForGlide(final Context context) {
        if (context == null) {
            return false;
        }
        if (context instanceof Activity) {
            final Activity activity = (Activity) context;
            if (activity.isDestroyed() || activity.isFinishing()) {
                return false;
            }
        }
        return true;
    }

definitely should not crash it, a log is enough

Why don't you have a config variable for Glide in the app to tell which behaviour it should have?

e.g.

a) default setting - the simple version for people who want a single code line to load an image without thinking about all other potential issues in more complex situations

b) current behaviour, loading of image is not stopped automatically and 20 additional code lines are required at some place in your own code

It sounds like what you're getting at is that I can fix the issue on my end instead of making a change in the library. However I do not believe this is the best solution since people use a simplified library like Glide because they don't have to worry about corner cases like this. In my opinion Glide should handle this directly. Obviously I'm no expert on Glide's code so I could be off base here.

To answer your question a network call comes in which then posts to run the code on the UI thread. Since I am in a view there I can do:

onAttachedToWindow
    isAttached = true;

onDetachedFromWindow
    isAttached = false;

// in my method i can call
if (isAttached) {
    //Glide call
}

What do you guys think?

Thank you so much. saved my time. 👍

So until today there is still no proper fix on Glide library side? I also still getting a lot of crash report with this even handling it side by side. The context may not be null but if the activity gets destroyed then boom app will still close. If I were a user of the app that would be unpleasant experience because not all user will comprehend what happened. We do understand the reason of throwing exception and its good, but Android platform is too fragmented where each device behaves differently depending on what modification on OS manufacturer does. At least add configuration like cancelOnDestroy(true) or just log.wtf it because in one app there will be hundreds scenario of needing to load an images and no one would really not want to do try catch on each of those request hahaha that would be a huge boilerplate. Try catch is said to be use for unexpected behavior where we do not have any control over that situation like slow internet connection but also expensive as its redirect the system flow or behavior.

how about crashing only in debug builds?

yes, this problem is often an error on the part of the programmer, but as evidenced by the numerous issues this can, if rarely, happen even when your code is correct.

it is not reasonable for programmers to expect that Glide will crash in the case when an activity is destroyed. it is also not reasonable for programmers to be aware of all the corner cases where their code calling Glide is called after the destruction of an activity. in other words, a good programmer can have their good code crash in production because Glide thinks that it is reasonable to crash in expected circumstances.

I have solved my problem using below method. May it help someone. I m using Glide to load image in Runnable Using Handler.

private boolean isPause = false, isDestroy = false;

 @Override
    public void onPause() {
        super.onPause();
        isPause = true;
        handler.removeCallbacksAndMessages(null);
    }

@Override
    public void onDestroy() {
        super.onDestroy();
        isDestroy = true;
        handler.removeCallbacksAndMessages(null);
    }


@Override
    public void onViewCreated(@NonNull View rootview, @Nullable Bundle savedInstanceState) {
        super.onViewCreated(rootview, savedInstanceState);
        ....
        if (!isPause && !isDestroy) {
                            Glide.with(rootview)
                                    .load("https://dummyimage.com/150x150/000000/ffffff.png&text=This+is+Image")
                                    .fitCenter()
                                    .placeholder(R.mipmap.ic_launcher)
                                    .error(R.mipmap.ic_launcher)
                                    .into(ivAdImage);
                        }

I'm using this context extension before loading:

/**
 * Returns true when [Context] is unavailable or is about to become unavailable
 */
fun Context?.isDoomed(): Boolean = when (this) {
    null -> true
    is Application -> false
    is Activity -> (this.isDestroyed or this.isFinishing)
    else -> false
}

This crash is affecting my app where...

• The app is mainly Jetpack Compose
• The app preloads ~100 small images when it is first launched with the following code:

allImageUrls.forEach {
  Glide.with(activity)
    .load(it)
    .priority(Priority.LOW)
    .diskCacheStrategy(DiskCacheStrategy.ALL)
    .preload()
}

How can I catch this exception so the app doesn't crash? It seems like, even if I switch this to do them one-by-one, it can still crash after Glide.with(activity) evaluates successfully but it is still in the loading / caching stage?