Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting IllegalArgument exception several times #23

Closed
lalith-b opened this issue Sep 26, 2013 · 17 comments
Closed

Getting IllegalArgument exception several times #23

lalith-b opened this issue Sep 26, 2013 · 17 comments
Labels

Comments

@lalith-b
Copy link

When I scroll the listview (with a 4 images in a row) up and down for a while it throws me this error.

09-26 14:32:31.233: E/AndroidRuntime(15158): FATAL EXCEPTION: bg_thread
09-26 14:32:31.233: E/AndroidRuntime(15158): java.lang.IllegalArgumentException: Problem decoding into existing bitmap
09-26 14:32:31.233: E/AndroidRuntime(15158):    at android.graphics.BitmapFactory.decodeStream(BitmapFactory.java:502)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at com.bumptech.glide.resize.load.Downsampler.decodeStream(Downsampler.java:182)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at com.bumptech.glide.resize.load.Downsampler.downsampleWithSize(Downsampler.java:114)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at com.bumptech.glide.resize.ImageManager$1.downsample(ImageManager.java:78)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at com.bumptech.glide.resize.load.ImageResizer.load(ImageResizer.java:113)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at com.bumptech.glide.resize.load.ImageResizer.load(ImageResizer.java:101)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at com.bumptech.glide.resize.ImageManager$ImageManagerRunner.getFromDiskCache(ImageManager.java:527)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at com.bumptech.glide.resize.ImageManager$ImageManagerRunner.run(ImageManager.java:510)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at android.os.Handler.handleCallback(Handler.java:605)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at android.os.Handler.dispatchMessage(Handler.java:92)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at android.os.Looper.loop(Looper.java:137)
09-26 14:32:31.233: E/AndroidRuntime(15158):    at android.os.HandlerThread.run(HandlerThread.java:60)
@sjudd
Copy link
Collaborator

sjudd commented Sep 26, 2013

If you're using a jar, what version are you using? If not, have you pulled forward recently?

This looks similar to: #19

@lalith-b
Copy link
Author

using glide-2.0.1.jar

I will try cloning the source from repo and use this and see if I'm able to reproduce this.

@csobrinho
Copy link
Contributor

Hi @sjudd . I still have this error some times. I have a mixture of PNG-24, indexed PNG-8 and JPGs all at the same time in the memory cache and pool.

After a little time scrolling in a ListView with a lot of small images I get the IllegalArgumentException already mentioned here and in issue #19.

Is it possible that there is still a bug in the config code or just a native exception that sometimes triggers? I believe there is a problem when either the memory cache or the pool is full or almost full.

@sjudd
Copy link
Collaborator

sjudd commented Oct 10, 2013

Ok good to know that it's not fixed in master. I'm pretty sure I know what causes this. You get this exception if you try to decode an image into a Bitmap and their configs are not identical. We go to some length to prevent this in the pool by including the config as part of the key (along with width and height).

However, BitmapFactory.decodeStream sometimes returns Bitmaps where getConfig() == null. This happens when the actual config of the bitmap doesn't match up with any of the configs in the enum (ie it's not RGB_565, ARGB_8888 etc). At this point, the bitmap pool treats all Bitmaps with config == null as having the same config. I suspect that there are actually multiple different configs that aren't in the enum and therefore are represented as null.

If you load enough images of enough different types, you will get unlucky and add two Bitmaps with different underlying configs to the same null config queue in the bitmap pool. When you try to decode using one of them, you get unlucky again, the configs don't match, and you get an exception.

I think the solution is to just exclude Bitmaps with getConfig() == null from the bitmap pool as we were before. The commit that should be added again is here: c256841

Can you try readding the lines in that commit and seeing if it fixes the exceptions you're seeing?

@csobrinho
Copy link
Contributor

Will try that tomorrow if I have time. On a tight schedule until Tuesday.

@lalith-b
Copy link
Author

when loading from diskcache this fails and crashes with the above log. :( duh..!

@lalith-b
Copy link
Author

I found the reason for this illegalArgument exception after digging through your code.

private Bitmap getFromDiskCache(String key) {
       Bitmap result = null;
       final InputStream is = diskCache.get(key);
       if (is != null) {
            result = resizer.load(is, width, height, DISK_CACHE_DOWNSAMPLER);
            if (result == null) {
                   diskCache.delete(key); //the image must have been corrupted
            }
       }
      return result;
}

the above method checks for result==null if I download a js file using Glide.load(jsfileurl).into(imageView) then there will be an error. But the disc cache key is created, and when reloading it it is not null.

private void putInDiskCache(String key, final Bitmap bitmap) {
     diskCache.put(key, new DiskCache.Writer() {
        @Override
         public void write(OutputStream os) {
            bitmap.compress(bitmap.getConfig() == Bitmap.Config.ARGB_8888 ? Bitmap.CompressFormat.PNG :      Bitmap.CompressFormat.JPEG, bitmapCompressQuality, os);
        }
      });
}

the above method doesn't check for bitmap.getConfig() !=null is that an issue ?

@sjudd
Copy link
Collaborator

sjudd commented Oct 15, 2013

Regarding the illegal argument exception: The get method you pasted isn't the cause. First, we only ever write bitmaps to the disk cache. If we did a get and got html/js, as you suggest, there would be an exception trying to decode that into a bitmap, which would prevent us from ever trying to write the value to the cache. Second, the null check in get is not checking the actual value in the disk cache, but rather the bitmap we try to decode from the value in the disk cache. If we managed to write js to the disk cache, result would still be null because we again wouldn't be able to decode it into a bitmap.

Regarding bitmap.getConfig(): That isn't really correct, you're right. Carlos has a pull request up that fixes the issue. However, the worst behavior i've seen from that is degraded image quality, it shouldn't cause an exception.

Thanks for taking a look at the code, I appreciate the suggestions. Did you try adding the lines in the commit I mentioned earlier? Did they solve and/or reduce the number of exceptions you're seeing?

@lalith-b
Copy link
Author

I found out the reason for the crash. Its the

com.bumptech.glide.resize.load.Downsampler.downsampleWithSize(Downsampler.java:114)

protected Bitmap downsampleWithSize(RecyclableBufferedInputStream bis, BitmapFactory.Options options,  BitmapPool pool, int inWidth, int inHeight, int sampleSize) {
     if (sampleSize > 1) {
        options.inSampleSize = sampleSize;
    } else {
        setInBitmap(options, pool.get(inWidth, inHeight, getConfig(bis)));
    }
    return decodeStream(bis, options);
}

The method has RecyclableBufferedInputStream as a parameter, in my situation I get images as well as some random text files/javascripts etc. With the code above it assumes and tries to decode a null bis. Also the getConfig(bis) returns a default ARG545 Config I think.

Due to the above reasons there was an illegalarg.

The IllegalArgs is still not solved though..! I pulled the code from master, and the config ! null check is there, But still the issue is not solved yet.

@csobrinho
Copy link
Contributor

Hi @sjudd, any update about this issue? I'll have some time Monday and/or Tuesday to test some patches...

@sjudd
Copy link
Collaborator

sjudd commented Oct 20, 2013

@deathlord87 I actually removed the != null check in the bitmap pool in master after making some other changes I had hoped would fix the problem (ie related to issue #19). You can see that the != null check is not present in master at https://github.com/bumptech/glide/blob/master/library/src/com/bumptech/glide/resize/bitmap_recycle/LruBitmapPool.java#L24 or at https://github.com/bumptech/glide/blob/master/library/src/com/bumptech/glide/resize/bitmap_recycle/LruBitmapPool.java#L126.

@csobrinho @deathlord87, If either or both of you could try cherry picking that commit I referenced above or otherwise inserting that != null check (right before line 24 is probably appropriate) and let me know if you still see the exception that would be helpful.

If you can reliably reproduce the issue, or can provide some links to images or images that seem to produce this or any other problem, that would be hugely helpful. Without test cases it will be hard for me to know whether or not I've fixed a problem without making you guys try a bunch of stuff :).

@csobrinho
Copy link
Contributor

I'm trying to check if the != null solved the issue. Will keep you posted.

@sjudd
Copy link
Collaborator

sjudd commented Nov 5, 2013

Awesome, thanks!

@sjudd
Copy link
Collaborator

sjudd commented Nov 7, 2013

Also this might have been the cause, it looks similar: #32

Are you guys loading gifs or tiffs or webp images?

@csobrinho
Copy link
Contributor

Nope. Only PNGs and JPGs but sometimes I also have Indexed PNGs. I had to add the try/catch to the Decoder method.

@lalith-b
Copy link
Author

lalith-b commented Jan 2, 2014

the try/catch works well, I sometimes try to load gifs/tiffs so was getting the IllegalArgumentException. It is solved by the catch clause.

@sjudd
Copy link
Collaborator

sjudd commented Mar 20, 2014

Hey guys, I know this is super old, but I think I may have fixed the issue here: f8a7681

Were you using approximate or asIs to load these images (or any transformation that didn't resize the bitmap to exactly matched the requested size)? If so, that commit probably fixed it. On the assumption you were, I'm going to close this issue for now, but please do reopen if you see it again after merging in that commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants