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

Glide behaves differently with support library 26.1.0 #2881

Closed
MFlisar opened this issue Feb 7, 2018 · 14 comments
Closed

Glide behaves differently with support library 26.1.0 #2881

MFlisar opened this issue Feb 7, 2018 · 14 comments

Comments

@MFlisar
Copy link

MFlisar commented Feb 7, 2018

Glide Version: 4.5

Integration libraries: none

Device/Android Version:

Issue details / Repro steps / Use case background:

Short introduction

I'm using glide and all my dependencies normally like following:

implementation ('com.github.bumptech.glide:glide:4.5.0'){
    exclude group: "com.android.support"
}

Then I add my own version to my project and am done. This works fine of course.

Currently I have an issue after upgrading from support library 26.1.0 to 27.0.2 (including target and build sdk 26 => 27). If I proguard my app I get Cannot instantiate class: android.support.v7.widget.SearchView exceptions (on some devices only) suddenly although my proguard file contains -keep class android.support.v7.widget.SearchView { *; }. I don't know how to solve this because imho this should be solved by the proguard line and it was solved like this in the past and I can't find anything new for this issue for the new support library or similar and can't solve this issue currently.

Summary

  • Support library 26.1.0:
    • RELEASE
      • I see problems with glide
    • DEBUG
      • everything works fine (no proguard)
  • Support library 27.0.2:
    • RELEASE
      • I have proguard issues (on some devices) with android.support.v7.widget.SearchView, so app crashes anyway
      • some devices (my own one for example) does not have issues and I can see glide works correct
    • DEBUG
      • everything works fine (no proguard)

Usage

I wanted to reuse my old code which worked with targets, so I created an InstantTarget to execute image loads synchronously (class is here: https://gist.github.com/MFlisar/5808cba2d6d970552e67d862a715e981).

I use it like following (ALL my requests run through this final function):

private static void loadIcon(RequestBuilder<Drawable> request, Target<Drawable> target) {
    request.listener(new LoggingListener());
    if (target instanceof InstantTarget) {
        FutureTarget t = null;
        try {
            t = request.submit(((InstantTarget) target).getWidth(), ((InstantTarget) target).getHeight());
            Drawable d = (Drawable) t.get();
            ((InstantTarget) target).setDrawable(d);
        } catch (InterruptedException e) {
            L.e(e);
        } catch (ExecutionException e) {
            L.e(e);
        } finally {
            if (t != null) {
                clear(t);
            }
        }
    } else {
        request.into(target);
    }
}

And my loader looks like following:

@Override
public void loadData(@NonNull Priority priority, @NonNull DataCallback<? super InputStream> callback) {
    try {
        if (mCancelled) {
            callback.onDataReady(null);
        } else {
            callback.onDataReady(loadData());
        }
    } catch (Exception e) {
        L.e(e, "LoadDataException: Class = %s", model.getClass());
        callback.onLoadFailed(e);
    }
}

private InputStream loadData() {
	// Setup
	...
	
	// Load
	InstantTarget instantTarget = new InstantTarget(w, h)
			.withUnlimitedPool();
	List<Drawable> drawables = new ArrayList<>();
	for (IFolderItem item : model.items) {
		ImageManager.loadSidebarItemIcon(item, instantTarget);
		Drawable d = instantTarget.getDrawable();
		drawables.add(d);
	}
	
	// Draw all images to one single bitmap
	
	// return bitmap as stream
	
	return ...;
}

Problem

When I use support library 26.1.0 and make a release build, I see following behaviour: all my nested reqeusts fail with a TimeoutException (what they don't do in debug!). Exception looks like following:

Request threw uncaught throwable
	java.lang.AssertionError: java.util.concurrent.TimeoutException
		at com.bumptech.glide.request.RequestFutureTarget.get(SourceFile:117)
		at com.my.app.images.ImageManager.a(SourceFile:430)
		at com.my.app.images.ImageManager.a(SourceFile:12540)
		at com.my.app.dummies.BaseDummyItem.a(SourceFile:34)
		at com.my.app.images.ImageManager.a(SourceFile:2109)
		at com.my.app.images.glide.loader.FolderItemLoader$FolderItemDataFetcher.e(SourceFile:116)
		at com.my.app.images.glide.base.BaseSidebarItemDataFetcher.a(SourceFile:81)
		at com.bumptech.glide.load.engine.SourceGenerator.a(SourceFile:62)
		at com.bumptech.glide.load.engine.DecodeJob.g(SourceFile:299)
		at com.bumptech.glide.load.engine.DecodeJob.run(SourceFile:1269)
		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:762)
		at com.bumptech.glide.load.engine.executor.GlideExecutor$DefaultThreadFactory$1.run(SourceFile:446)
	 Caused by: java.util.concurrent.TimeoutException
		at com.bumptech.glide.request.RequestFutureTarget.a(SourceFile:216)
		at com.bumptech.glide.request.RequestFutureTarget.get(SourceFile:115)
		at com.my.app.images.ImageManager.a(SourceFile:430) 
		at com.my.app.images.ImageManager.a(SourceFile:12540) 
		at com.my.app.dummies.BaseDummyItem.a(SourceFile:34) 
		at com.my.app.images.ImageManager.a(SourceFile:2109) 
		at com.my.app.images.glide.loader.FolderItemLoader$FolderItemDataFetcher.e(SourceFile:116) 
		at com.my.app.images.glide.base.BaseSidebarItemDataFetcher.a(SourceFile:81) 
		at com.bumptech.glide.load.engine.SourceGenerator.a(SourceFile:62) 
		at com.bumptech.glide.load.engine.DecodeJob.g(SourceFile:299) 
		at com.bumptech.glide.load.engine.DecodeJob.run(SourceFile:1269) 
		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:762) 
		at com.bumptech.glide.load.engine.executor.GlideExecutor$DefaultThreadFactory$1.run(SourceFile:446)

Any ideas how I could solve this issue? Or why I see different behaviour between debug and release builds? Glide only uses the SupportFragment from the support libraries, correct? There should not be any issues just because I use 26.1.0 support library

@sjudd
Copy link
Collaborator

sjudd commented Feb 8, 2018

Support library issues sound like #2730. The timeout exception may be happening if you're nesting requests because doing so can lead to deadlock depending on the number of threads Glide has available. If you want to nest requests, make sure your nested requests always apply useUnlimitedSourceGeneratorsPool() to avoid the deadlock.

@sjudd sjudd added the question label Feb 8, 2018
@MFlisar
Copy link
Author

MFlisar commented Feb 8, 2018

  • I'm excluding the support library from glide (and I do this from other libraries I use as well always anyways)
  • I'm using useUnlimitedSourceGeneratorsPool() for all nested requests, not for others though
  • I don't have issues with any configuration (support lib v26.1.0 nor v27.0.2) in debug builds => ALL nested requests work without a problem, but I do have issues in my release build if I use support lib v26.1.0 => ALL nested requests fail

I don't have anything in code that differentiates between debug and release builds, still all nested glide reqeusts fail if I use support lib v26.1.0 in release and all nested glide requests succeed if I simply build the app in debug...

@MFlisar
Copy link
Author

MFlisar commented Feb 9, 2018

I've finally removed my usage of SearchView in my app and everything is working now as I'm using support library 27.0.2.

Now I have found out that I can reproduce the TimeoutException in this configuration as well. It really looks like proguard is removing something that it should not, but based on the exception I don't know what.

proguard setup

-keep public class * implements com.bumptech.glide.module.GlideModule
-keep public class * extends com.bumptech.glide.module.AppGlideModule
-keep public enum com.bumptech.glide.load.resource.bitmap.ImageHeaderParser$** {
  **[] $VALUES;
  public *;
}

# Issue: https://github.com/bumptech/glide/issues/2807
-dontwarn com.bumptech.glide.load.resource.bitmap.VideoDecoder

rules

  • proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro'

    => all nested requests lead to TimeoutException (same stacktrace as above)

  • proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro'

    => all nested requests succeed without a problem

All I do is changing the above proguard rules from default to optimised and I already see the issue. I compiled my app with each config twice and always see the same results...

@sjudd
Copy link
Collaborator

sjudd commented Feb 9, 2018

What is in proguard-android-optimize? What's weird is it doesn't look like you're using the get() method in RequestFutureTarget that supports a timeout.

Interestingly TimeoutException is thrown more or less as a default exception if we attempt to wait for a result and our waiter finishes without receiving either a result or an error:

if (timeoutMillis == null) {
waiter.waitForTimeout(this, 0);
} else if (timeoutMillis > 0) {
long now = System.currentTimeMillis();
long deadline = now + timeoutMillis;
while (!isDone() && now < deadline) {
waiter.waitForTimeout(this, deadline - now);
now = System.currentTimeMillis();
}
}
if (Thread.interrupted()) {
throw new InterruptedException();
} else if (loadFailed) {
throw new ExecutionException(exception);
} else if (isCancelled) {
throw new CancellationException();
} else if (!resultReceived) {
throw new TimeoutException();
}
return resource;

I wonder if there's an optimization that's stripping the while loop?

@MFlisar
Copy link
Author

MFlisar commented Feb 9, 2018

  • I'm using the official optimised proguard file here: https://android.googlesource.com/platform/sdk/+/master/files/proguard-android-optimize.txt
  • I'm using FutureTarget t = request.submit(width, height).get() so this should use the code you posted
  • I don't think the loop is stripped out, because the experience I make is that my app blocks all image loads for a minute or a little longer and then I see the exceptions in my logcat and only afterwards I see the simple images loaded. So the timeout seems to work...

@MFlisar
Copy link
Author

MFlisar commented Feb 9, 2018

Do I have to make sure that my nested requests are thread safe? Could this be a problem? Could those lock out each other? My app will likely try to load multiple requests with sub requests at once, is glide taking care of this?

@stale
Copy link

stale bot commented Feb 16, 2018

This issue has been automatically marked as stale because it has not had activity in the last seven days. It will be closed if no further activity occurs within the next seven days. Thank you for your contributions.

@stale stale bot added stale and removed stale labels Feb 16, 2018
@stale
Copy link

stale bot commented Feb 24, 2018

This issue has been automatically marked as stale because it has not had activity in the last seven days. It will be closed if no further activity occurs within the next seven days. Thank you for your contributions.

@stale stale bot added the stale label Feb 24, 2018
@hannojg
Copy link

hannojg commented Feb 26, 2018

I am experiencing the same issue. Any resolution on it so far?
Have you discovered something @MFlisar?

the only difference between the optimized proguard file for android and the normal is that the optimized has the optimize parameter set, the other doesn't use optimization. So it is hard to determine what exactly is going wrong here :/

@stale stale bot removed the stale label Feb 26, 2018
@MFlisar
Copy link
Author

MFlisar commented Feb 26, 2018

Currently I've disabled minifyEnabled and shrinkResources and use the normal 'proguard-android.txt and not the optimised version...

In the end, it's not the optimised for me but something in proguard minifying my code, as far as I can tell. I only found one thing, that may help you:

Be careful with assumenosideeffects, this may strip away Object.wait and similar. Can't find the discussion anymore, but one of the proguard devs wrote in one github issue that this is a dangerous settings. Can only find following currently: https://stackoverflow.com/questions/16846691/proguard-ate-my-object-wait Not sure if this has any effect here, but it may be an issue.

Maybe this helps you, I had not yet time to investigate this deeper

Checkout library does use this potentially dangerous proguard rule, maybe you use it...

@MFlisar
Copy link
Author

MFlisar commented Feb 26, 2018

As you asked and I had sime time now it seems to be this issue for me.

My issue + solution is written here: https://github.com/serso/android-checkout/issues/139

After this change, I could just create an optimised proguarded app that loads nested images as well (I clear glide's cache therefore and test it this way).

That this is working for my device is no proof so far, if I remeber correctly, I had issues with some devices only, but I will try this new setup soon and check if this solves the issue on other devices as well

@hannojg
Copy link

hannojg commented Feb 27, 2018

Big thank you for the provided information! I ran into the same issue that I let proguard remove the Object.wait method though assumenosideeffects triggering this misbehaviour

@sjudd
Copy link
Collaborator

sjudd commented Feb 28, 2018

Sorry I missed this and thank you for the detailed follow up @MFlisar, super helpful!

@stale
Copy link

stale bot commented Mar 7, 2018

This issue has been automatically marked as stale because it has not had activity in the last seven days. It will be closed if no further activity occurs within the next seven days. Thank you for your contributions.

@stale stale bot added the stale label Mar 7, 2018
@stale stale bot closed this as completed Mar 14, 2018
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

3 participants