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

OOM on Glide error #491

Closed
Tolriq opened this issue Jun 10, 2015 · 20 comments
Closed

OOM on Glide error #491

Tolriq opened this issue Jun 10, 2015 · 20 comments

Comments

@Tolriq
Copy link
Contributor

Tolriq commented Jun 10, 2015

Don't know if it should be handled by default or offer an option for it, but Glide can currently OOM on decoding error / placeholder drawables.

Since this happens out of application control it's impossible for the application to catch that and react accordingly.

(I normally catch those OOM and clear backstack then glide cache to recover most of the time).

java.lang.OutOfMemoryError
       at android.graphics.BitmapFactory.nativeDecodeAsset(BitmapFactory.java)
       at android.graphics.BitmapFactory.decodeStream(BitmapFactory.java:557)
       at android.graphics.BitmapFactory.decodeResourceStream(BitmapFactory.java:394)
       at android.graphics.drawable.Drawable.createFromResourceStream(Drawable.java:785)
       at android.content.res.Resources.loadDrawable(Resources.java:2016)
       at android.content.res.ResourcesEx.loadDrawable(ResourcesEx.java:580)
       at android.content.res.Resources.getDrawable(Resources.java:697)
       at com.bumptech.glide.request.GenericRequest.onException(GenericRequest.java:548)
       at com.bumptech.glide.load.engine.EngineJob.access$200(EngineJob.java:22)
       at com.bumptech.glide.load.engine.EngineJob$MainThreadCallback.handleMessage(EngineJob.java:204)
       at android.os.Handler.dispatchMessage(Handler.java:103)
       at android.os.Looper.loop(Looper.java:194)
       at android.app.ActivityThread.main(ActivityThread.java:5468)
       at java.lang.reflect.Method.invokeNative(Method.java)
       at java.lang.reflect.Method.invoke(Method.java:525)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:936)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:703)
       at dalvik.system.NativeStart.main(NativeStart.java)
@TWiStErRob
Copy link
Collaborator

Does #435's fix help you?

@Tolriq
Copy link
Contributor Author

Tolriq commented Jun 10, 2015

Well no it does not as it's to LOG by default and OOM are a little different to handle in Java.

@TWiStErRob
Copy link
Collaborator

Oh, sorry I thought there was a custom handler too.

@TWiStErRob
Copy link
Collaborator

Can't you use THROW and/or Thread.UncaughExceptionHandler? ... Oh, maybe onException is not covered by #435.

@Tolriq
Copy link
Contributor Author

Tolriq commented Jun 10, 2015

You can configure it but if it's LOG by default and it crash, then the handler will obviously not be called in case of OOM as to be called it would have need to be catched ;)

@TWiStErRob
Copy link
Collaborator

... "and that's a bad miss", good point. :)

@TWiStErRob
Copy link
Collaborator

@sjudd could add more defensive catches, but you should be able to catch it in the meantime with Thread.UncaughtExceptionHandler.

@Tolriq
Copy link
Contributor Author

Tolriq commented Jun 10, 2015

Yes I can do lot's of things, but I also have things already handled + Crashlytics + + + +, and some OOM should still crash when on other part of the application since not recoverable.

@sjudd
Copy link
Collaborator

sjudd commented Jun 11, 2015

How large are the assets you're using for placeholders?

If they're not particularly large, you probably have a memory leak, in which case try/catching around the placeholders won't do you much good, you will just OOM soon after somewhere else randomly.

@Tolriq
Copy link
Contributor Author

Tolriq commented Jun 11, 2015

Some can be large but please read again the first part of what I do on OOM :)

My app can have a large back stack depending on how the user navigate and on some devices this can be problematic, catching those and clearing backstack does recover most of the time, this is not related to memory leaks and does not remove the underlying need :)

Usually it's more rob that tells me you're doing it wrong do it another way ;)

@sjudd
Copy link
Collaborator

sjudd commented Jun 13, 2015

Hah sorry, I don't mean to tell you how to write your app. My concern is mostly that it's hard to know what to do when if we do catch an OOM creating a placeholder. We can't call any of the normal error methods, because those typically also show a placeholder, so we will end up in a loop. We could add an additional failure method, but that seems redundant and confusing.

I don't disagree that this could be useful, I'm just not sure I see a straightforward solution.

@Tolriq
Copy link
Contributor Author

Tolriq commented Jun 13, 2015

Can't 2cf8671 be extended to support OOM ? Just need a new specific catch block as it's not an exception but still a throwable if it's the correct place.

@sjudd
Copy link
Collaborator

sjudd commented Jun 17, 2015

Unfortunately not, that captures exceptions thrown by the decode process on a background executor. The placeholders are retrieved from resources on the main thread.

We could just set null placeholders if we catch an OOM, but then you'd have to infer that receiving a null placeholder in the Target when a placeholder was specified actually indicated an OOM, which is a little odd.

@bezineb5
Copy link

bezineb5 commented Nov 1, 2015

Hi,
I had the same issue and replaced Exception by Throwable in the handlers: bezineb5@4d77c95
It allows you to catch OOM in the onFailed callback. The problem is that it's a breaking change and you'll have to update all the signatures of the methods overriding BaseTarget.onLoadFailed.

I hope this helps!

@tibbi tibbi mentioned this issue Jan 4, 2016
@TWiStErRob
Copy link
Collaborator

@bezineb5 you can do it without changing signatures, see #859.

@4sskick
Copy link

4sskick commented Mar 31, 2016

Hi, I need some advice about handling OutOfMemoryError. I think this should be caused by memory leaks I just call like this

Glide.with(context)
                .load(event.getImageUrl())
                .diskCacheStrategy(DiskCacheStrategy.ALL)
                .placeholder(R.drawable.placeholder)
                .into(eventImage);

the resource file placeholder is about 516 x 272 px and I think this is not large anyway

03-31 23:27:03.698 10147-10147/me.gilo.eventhub E/AndroidRuntime: FATAL EXCEPTION: main
Process: me.gilo.eventhub, PID: 10147
java.lang.OutOfMemoryError: Failed to allocate a 10890012 byte allocation with 4194304 free bytes and 10MB until OOM
  at dalvik.system.VMRuntime.newNonMovableArray(Native Method)
  at android.graphics.BitmapFactory.nativeDecodeAsset(Native Method)
  at android.graphics.BitmapFactory.decodeStream(BitmapFactory.java:609)
  at android.graphics.BitmapFactory.decodeResourceStream(BitmapFactory.java:444)
  at android.graphics.drawable.Drawable.createFromResourceStream(Drawable.java:1080)
  at android.content.res.Resources.loadDrawableForCookie(Resources.java:2635)
  at android.content.res.Resources.loadDrawable(Resources.java:2540)
  at android.content.res.TypedArray.getDrawable(TypedArray.java:870)
  at android.widget.ImageView.<init>(ImageView.java:152)
  at android.widget.ImageView.<init>(ImageView.java:140)
  at android.support.v7.widget.AppCompatImageView.<init>(AppCompatImageView.java:58)
  at android.support.v7.widget.AppCompatImageView.<init>(AppCompatImageView.java:54)

@TWiStErRob
Copy link
Collaborator

@4sskick please do some more investigation of your heap: @Tolriq here doesn't have memory leaks, he just has a deep stack and active images accumulate (they're all displayed in the background behind the visible activity, this is an Android limitation).

@4sskick Please open a new issue if you want to us to help investigating. Include what you find out when doing: Android Studio > Android Monitor > Memory > Dump Java Heap > Analyzer Tasks > Perform Analysis > Leaked Activities

@Tolriq you might be interested in #1057.

@TWiStErRob TWiStErRob removed the bug label Jul 11, 2016
@andreaboi83
Copy link

Hi! It would be nice if we could set some special flags on Bitmap decode options, like inPurgeable. I know it's deprecated (since Lollipop), but in our app it was the only way to avoid the OOMs, expecially in some device with Dalvik VM (where the Bitmaps sometimes can't be GC, unless you set inPurgeable).

I don't know if that can be done with a custom Downsampler, like described in issue #1048...

@TWiStErRob
Copy link
Collaborator

@andreaboi83 your best bet without modifying the source of Glide is to hack Downsampler.OPTIONS_QUEUE with reflection to an instance you control and always return an object from poll (see Downsampler.getDefaultOptions). A better way would be to refactor Downsampler so that it's possible to extend it how you want (there are a lot of privates and statics currently) and create PR. Alternatively you can override the unrelated getDimensions method which sets the fields you need.

@sjudd
Copy link
Collaborator

sjudd commented Nov 12, 2017

We handle OOMs on Glide's threads now so that they're passed back to requests. We don't have way to handle OOMs on placeholders. If someone has an idea of how that could be done coherently, please open a new issue. At the moment we'd have to just silently not show the placeholder. Given that OOMs are usually caused by other issues that are fixable, that seems like a weird default.

@sjudd sjudd closed this as completed Nov 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants