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

It's maybe a bug for clear target by using okhttp3 #3343

Open
zaydenyang opened this issue Oct 8, 2018 · 0 comments
Open

It's maybe a bug for clear target by using okhttp3 #3343

zaydenyang opened this issue Oct 8, 2018 · 0 comments
Labels

Comments

@zaydenyang
Copy link

zaydenyang commented Oct 8, 2018

0. the problem:

The function of GlideApp.with(context).clear(target); was invoked when I try to cancel a url-request. It expected that the url-request was canceled but I found the callback function of target which is named onLoadCleared() worked while the url-request was still in the queue of okhttp3 and it still could load the bitmap from internet. I think it seemed to be a bug.

1. my dependencies:

    implementation 'com.squareup.okio:okio:2.1.0'
    implementation 'com.squareup.okhttp3:okhttp:3.11.0'
    implementation 'com.jakewharton:disklrucache:2.0.2'
    implementation 'com.github.bumptech.glide:glide:4.8.0'
    annotationProcessor 'com.github.bumptech.glide:compiler:4.8.0'
    implementation 'com.github.bumptech.glide:okhttp3-integration:4.8.0'

2. my test code:

    // DemoAppGlideModule.java
    @Override
    public void registerComponents(@NonNull Context context, @NonNull Glide glide, @NonNull Registry registry) {
        super.registerComponents(context, glide, registry);
        OkHttpClient client = new OkHttpClient.Builder()
                .connectTimeout(5, TimeUnit.SECONDS)
                .readTimeout(10, TimeUnit.SECONDS)
                .writeTimeout(10, TimeUnit.SECONDS)
                .retryOnConnectionFailure(false)
                .build();
        registry.replace(GlideUrl.class, InputStream.class, new OkHttpUrlLoader.Factory(client));
    }

   // MainActivity.java
    @Override
    protected void onResume() {
        super.onResume();
        Random random = new Random();
        new Thread() {
            @Override
            public void run() {
                super.run();
                // pathSet contains some urls
                Iterator<String> iterator = pathSet.iterator();
                while (iterator.hasNext()) {
                    final String path = iterator.next();
                    // DemoGlideTarget extends SimpleTarget<Drawable>, do nothing but just show log of callback 
                    final DemoGlideTarget target = new DemoGlideTarget(500, 500, path, null, false);
                    UIHANDLER.post(new Runnable() {
                        @Override
                        public void run() {
                            GlideApp.with(MainActivity.this).load(path).skipMemoryCache(true).diskCacheStrategy(DiskCacheStrategy.NONE).into(target);
                        }
                    });
                    UIHANDLER.postDelayed(new Runnable() {
                        @Override
                        public void run() {
                            GlideApp.with(MainActivity.this).clear(target);
                        }
                    }, 60 + random.nextInt(500));

                    try {
                        Thread.sleep(random.nextInt(100));
                    } catch (Throwable e) {
                    }
                }
            }
        }.start();
    }

3. my analyze:

When use registry.replace(GlideUrl.class, InputStream.class, new OkHttpUrlLoader.Factory(client));, the MultiModelLoader.MultiFetcher.fetchers contains UrlUriLoader's fetcher and HttpUriLoader's fetcher, so it will create two OkHttpStreamFetcher for each of the url-requests, and the UrlUriLoader's fetcher excutes first. When cancel(clear target) occurs, the OkHttpStreamFetcher.cancel() works, and then comes OkHttpStreamFetcher.onFailure("Cancle"). Eventually MultiFetcher.startNextOrFail() will be called and the HttpUriLoader's fetcher will enqueue a new request into okhttp3's queue although the url-request should have been canceled.

Please check for that?

@sjudd sjudd added the bug label Oct 9, 2018
sjudd added a commit to sjudd/glide that referenced this issue Oct 15, 2018
There's a race described in bumptech#3343 where a child DataFetcher can call the MultiModelLoader after it has been cancelled, which can result in us unnecessarily starting a new load. Although we should probably also change the fetcher to avoid calling its callback after it's cancelled, we can make this behavior less of a problem in general by changing MultiModelLoader instead of relying on every individual DataFetcher implementation.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=216546841
sjudd added a commit to sjudd/glide that referenced this issue Dec 17, 2018
There's a race described in bumptech#3343 where a child DataFetcher can call the MultiModelLoader after it has been cancelled, which can result in us unnecessarily starting a new load. Although we should probably also change the fetcher to avoid calling its callback after it's cancelled, we can make this behavior less of a problem in general by changing MultiModelLoader instead of relying on every individual DataFetcher implementation.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=216546841
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

2 participants