bumptech / glide

An image loading and caching library for Android focused on smooth scrolling
https://bumptech.github.io/glide/
Other
34.47k stars 6.11k forks source link

NPE in SourceGenerator.onDataReady because of null DiskCacheStrategy #2967

Open elihart opened 6 years ago

elihart commented 6 years ago

Glide Version: 4.6.1

Integration libraries: OkHttp3

Device/Android Version: Seeing it in many models from Samsung, Huawei, and Xiaomi. In Android L, M, N

Issue details / Repro steps / Use case background:

I cannot reproduce this, but have several thousand occurrences in our crash monitoring. It is mainly happening on our list of search results where we have a RecyclerView with many images in it. I haven't been able to isolate any patterns besides that.

Glide load line / GlideModule (if any) / list Adapter code (if any):

Glide.with(imageView.context)
          .asBitmap()
           .load(url) // string
           // plus a bunch of logic for request options and nested thumbnails

Fairly complicated, but the hierarchy is basically:

Stack trace / LogCat:

java.lang.NullPointerException: Attempt to read from field 'com.bumptech.glide.load.data.DataFetcher com.bumptech.glide.load.model.ModelLoader$LoadData.fetcher' on a null object reference
        at com.bumptech.glide.load.engine.SourceGenerator.onDataReady(SourceGenerator.java:106)
        at com.bumptech.glide.load.model.MultiModelLoader$MultiFetcher.onDataReady(MultiModelLoader.java:133)
        at com.bumptech.glide.integration.okhttp3.OkHttpStreamFetcher.onResponse(OkHttpStreamFetcher.java:71)
        at okhttp3.RealCall$AsyncCall.execute(RealCall.java:141)
        at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1133)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:607)
        at java.lang.Thread.run(Thread.java:761)
Stev3nsen commented 6 years ago

Hey, i got a similar error because of a null DiskCacheStrategy.

GlideApp.with(this)
  .load(url)
  .diskCacheStrategy(DiskCacheStrategy.NONE)
  .transition(withCrossFade())
  .listener(new ErrorLogListener())
  .error(GlideApp.with(icon).load(url).error(R.drawable.ic_not_interested_white_48dp))
  .into(icon);
java.lang.NullPointerException: Attempt to invoke virtual method 'boolean com.bumptech.glide.load.engine.DiskCacheStrategy.isDataCacheable(com.bumptech.glide.load.DataSource)' on a null object reference
    at com.bumptech.glide.load.engine.SourceGenerator.onDataReady(SourceGenerator.java:106)
    at com.bumptech.glide.load.model.MultiModelLoader$MultiFetcher.onDataReady(MultiModelLoader.java:133)
    at de.s4f.extensions.volley.CachedByteArrayRequest.deliverResponse(CachedByteArrayRequest.java:47)
    at de.s4f.extensions.volley.CachedByteArrayRequest.deliverResponse(CachedByteArrayRequest.java:14)
    at com.android.volley.ExecutorDelivery$ResponseDeliveryRunnable.run(ExecutorDelivery.java:106)
    at android.os.Handler.handleCallback(Handler.java:790)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loop(Looper.java:164)
    at android.app.ActivityThread.main(ActivityThread.java:6494)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:438)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:807)
sjudd commented 6 years ago

Let me know if you find a way to reproduce this.

sjudd commented 6 years ago

Also @Zugreiter are you also using the okhttp3 integration library?

One way this could happen is if a DataFetcher notifies its callback after it has been cancelled.

elihart commented 6 years ago

I still can't reproduce this, but it is now one of our highest crashes.

For some reason it is much more common in our China builds (roughly 50x our rate elsewhere). No idea why this would be, but thought I would pass on that information in case it is useful.

Do you have any suggestions on things we could try to maybe work around this and reduce the crash rate?

elihart commented 6 years ago

@sjudd I thought more about your comment

One way this could happen is if a DataFetcher notifies its callback after it has been cancelled.

And realized we do have one custom DataFetcher, a base64 blurred image preview that we adopted from sample code from: https://github.com/TWiStErRob/glide-support/commit/e10619ef737d3adc60e19bf5fb224dc4c0d54055

our class

class Base64StreamDataFetcher implements DataFetcher<InputStream> {
    private final Base64Model base64;

    public Base64StreamDataFetcher(Base64Model base64) {
        this.base64 = base64;
    }

    @Override
    public void cancel() {

    }

    @Override
    public Class<InputStream> getDataClass() {
        return InputStream.class;
    }

    @Override
    public DataSource getDataSource() {
        return DataSource.REMOTE;
    }

    @Override
    public void loadData(Priority priority, DataCallback<? super InputStream> callback) {
        if (base64.isEmpty()) {
            callback.onDataReady(null);
        }
        callback.onDataReady(new ByteArrayInputStream(Base64.decode(base64.getBase64(), Base64.DEFAULT)));
    }

    @Override
    public void cleanup() {

    }
}

Are you suggesting that we should change our loadData implementation to not call callback.onDataReady after cancel has been called?

elihart commented 6 years ago

actually I guess our Base64 loader would be unrelated since the stacktrace shows that it is the OkHttpStreamFetcher