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 in Fragment error: recursive entry to executePendingTransactions #117

Closed
okrosa opened this issue Sep 5, 2014 · 5 comments · Fixed by #120
Closed

Glide in Fragment error: recursive entry to executePendingTransactions #117

okrosa opened this issue Sep 5, 2014 · 5 comments · Fixed by #120
Assignees
Labels
Milestone

Comments

@okrosa
Copy link

okrosa commented Sep 5, 2014

I have a customized ImageView which is using Glide to load up it's content during onAttachedToWindow(). It works correctly if it is embedded into an activity, however if I put it into a Fragment it fails.
I use Glide with this two lines as recommended:

Glide.get(getContext()).register(GlideUrl.class, InputStream.class, new VolleyUrlLoader.Factory(...));
Glide.with(getContext()).load(...).into(this);

It fails on the second line, and note that in the Fragment's case, the getContext() is returning the FragmentActivity, not the Fragment. I'm guessing that this is the problem, which I might be able to circumvent with some hacking (by providing the Fragment to the view, which I don't really want to do).
Anyway the resulting stack trace is the following:

FATAL EXCEPTION: main
java.lang.IllegalStateException: Recursive entry to executePendingTransactions
    at android.support.v4.app.FragmentManagerImpl.execPendingActions(FragmentManager.java:1456)
    at android.support.v4.app.FragmentManagerImpl.executePendingTransactions(FragmentManager.java:482)
    at com.bumptech.glide.manager.RequestManagerRetriever.supportFragmentGet(RequestManagerRetriever.java:148)
    at com.bumptech.glide.manager.RequestManagerRetriever.get(RequestManagerRetriever.java:66)
    at com.bumptech.glide.manager.RequestManagerRetriever.get(RequestManagerRetriever.java:29)
    at com.bumptech.glide.Glide.with(Glide.java:537)
@TWiStErRob
Copy link
Collaborator

I was trying to reproduce it in a test (see below), but it didn't happen.
However, I was able to reproduce it in one of my apps, by putting an ImageView like below into one of the fragment's XML. I'm not sure what the difference is, probably Robolectric.

As far as I know there's a difference between giving Fragment and FragmentActivity to with(), see where the RequestManagerRetriever.supportFragmentGet gets the FragmentManager from: getSupportFragmentManager() VS getChildFragmentManager().
The same conclusion is here on SO.

An ImageView which knows what to load is a little strange, you could try externalizing logic from the view and then you don't have to pass the Fragment in, but I'll let @sjudd weigh in on the right solution.

private static class Issue117Activity extends FragmentActivity {
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        int id = View.generateViewId();
        ViewGroup container = new FrameLayout(this);
        container.setId(id);
        setContentView(container, new ViewGroup.LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT));
        getSupportFragmentManager().beginTransaction().add(id, new Issue117Fragment()).commit();
    }

    private static class Issue117Fragment extends Fragment {
        @Override
        public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
            return new Issue117ImageView(inflater.getContext());
        }
    }

    private static class Issue117ImageView extends ImageView {
        public Issue117ImageView(Context context) {
            super(context);
        }

        @Override
        protected void onAttachedToWindow() {
            super.onAttachedToWindow();
            Glide.with(getContext()).load(android.R.drawable.ic_menu_rotate).into(this);
        }
    }
}

@Test
public void testIssue117() {
    Robolectric.buildActivity(Issue117Activity.class).create().start().resume().visible();
    Robolectric.shadowOf(Looper.getMainLooper()).runToEndOfTasks();
}

@okrosa
Copy link
Author

okrosa commented Sep 5, 2014

Thanks for the fast check&answer! This custom ImageView is showing profile images, can be swiped, etc, that's why I put the actual image loading inside it. (I was using Volley now, which cannot handle the exif, that's why I tried the Glide stuff.) Most certainly I could manage to get it working by some refactoring, however I still think it should not fail in this case.

@sjudd
Copy link
Collaborator

sjudd commented Sep 5, 2014

@TWiStErRob Thanks for weighing in and writing up that test Activity! You're also correct that Glide would work best if it had access to the Fragment containing the view, which means the ideal solution here is to externalize the logic related to doing the load so that it is done in the fragment.

All that being said, @okrosa I know subclassing views and using onAttachedToWindow and onDetachedFromWindow is a relatively common pattern and I agree we shouldn't crash if you try to use those to load images.

A workaround for right now is to simply pass in getContext().getApplicationContext() into Glide.with() rather than getContext() directly. Doing so will mean your load isn't paused or restarted or otherwise synced with your Fragment's lifecycle, but should work otherwise.

I'll work creating a project based on TWiStErRob's test above to reproduce the issue and try to come up with a permanent fix. Thanks for reporting the issue!

@sjudd sjudd self-assigned this Sep 5, 2014
sjudd added a commit to sjudd/glide that referenced this issue Sep 5, 2014
sjudd added a commit to sjudd/glide that referenced this issue Sep 5, 2014
sjudd added a commit to sjudd/glide that referenced this issue Sep 5, 2014
sjudd added a commit to sjudd/glide that referenced this issue Sep 6, 2014
@sjudd sjudd closed this as completed in #120 Sep 6, 2014
@TWiStErRob
Copy link
Collaborator

@okrosa if you don't want to wait for the next release:

repositories {
    maven { url "https://oss.sonatype.org/content/repositories/snapshots/" }
}
dependencies {
    compile 'com.github.bumptech.glide:glide:3.4.+'
}

@okrosa
Copy link
Author

okrosa commented Sep 6, 2014

Guys, thanks for the super fast feedbacks and the fix! I'll try the new version today in our project.

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

Successfully merging a pull request may close this issue.

3 participants