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

Crash on data-set update when DiffUtils is used #332

Closed
1 task done
saket opened this issue Jul 6, 2017 · 11 comments
Closed
1 task done

Crash on data-set update when DiffUtils is used #332

saket opened this issue Jul 6, 2017 · 11 comments
Labels

Comments

@saket
Copy link

saket commented Jul 6, 2017

Issues and steps to reproduce

I'm seeing this crash when trying to filter my list items using a search query:

FATAL EXCEPTION: main
java.lang.ArrayIndexOutOfBoundsException: length=45; index=-1
    at com.google.android.flexbox.FlexboxLayoutManager$AnchorInfo.assignFromView(FlexboxLayoutManager.java:2863)
    at com.google.android.flexbox.FlexboxLayoutManager$AnchorInfo.access$1900(FlexboxLayoutManager.java:2792)
    at com.google.android.flexbox.FlexboxLayoutManager.updateAnchorFromChildren(FlexboxLayoutManager.java:1136)
    at com.google.android.flexbox.FlexboxLayoutManager.updateAnchorInfoForLayout(FlexboxLayoutManager.java:1032)
    at com.google.android.flexbox.FlexboxLayoutManager.onLayoutChildren(FlexboxLayoutManager.java:700)
    at android.support.v7.widget.RecyclerView.dispatchLayoutStep2(RecyclerView.java:3583)
    at android.support.v7.widget.RecyclerView.dispatchLayout(RecyclerView.java:3312)
    at android.support.v7.widget.RecyclerView.onLayout(RecyclerView.java:3844)

Steps to reproduce
This started to happen after I added DiffUtils for showing item animations. Here's what I'm doing:

List<FolderSubscription> oldSubscriptions = ...;
List<FolderSubscription> newSubscriptions = ...;

FolderSubscriptionDiffCallbacks callback = new FolderSubscriptionDiffCallbacks(oldSubscriptions, newSubscriptions);
DiffUtil.DiffResult folderDiffResult = DiffUtil.calculateDiff(callback, true /* detectMoves */);

folderAdapter.updateData(newSubscriptions);
folderDiffResult.dispatchUpdatesTo(folderAdapter);

Version of the flexbox library

I tested on both v0.3-alpha3 and v0.3.

@thagikura
Copy link
Contributor

Thanks for the report. I'm not familiar with RxJava, it is possible replace the chain without RxJava?

@saket
Copy link
Author

saket commented Jul 10, 2017

Sorry for the late response. I have removed RxJava from my post.

@thagikura
Copy link
Contributor

Sorry for the late response.
This looks to be an issue with FlexboxLayoutManager.
Marking as a bug.

@thagikura thagikura added the bug label Jul 27, 2017
@saket
Copy link
Author

saket commented Jul 27, 2017

No problem, thanks @thagikura!

@silviorp
Copy link

I'm having the same issue using Renderers library and their diff algorithm with FlexboxLayoutManager.
The lib algorithm is the following:

  DiffCallback diffCallback = new DiffCallback(collection, newList);
  DiffUtil.DiffResult diffResult = DiffUtil.calculateDiff(diffCallback);
  clear();
  addAll(newList);
  diffResult.dispatchUpdatesTo(this);

Changing from their adapter.diffUpdate(list) to the conventional updating the list and calling adapter.notifyDataSetChanged(); works but I've lost the animation.

@derekcsm
Copy link

Would love to see DiffUtil support for animations as well

@Bringoff
Copy link

Bringoff commented Dec 26, 2017

@thagikura Any progress on this issue? I rely heavily on DiffUtils for proper animations
Method updateAnchorFromChildren in FlexboxLayoutManager contains such comment:

// If all visible views are removed in 1 pass, reference child might be out of bounds.
// If that is the case, offset it back to 0 so that we use these pre-layout children.

but out of bounds crash throws from the line before this comment: anchorInfo.assignFromView(referenceChild);
I suppose this might be the issue.

@Bringoff
Copy link

Bringoff commented Feb 16, 2018

@thagikura I've made a temp fix for myself in my fork by adding a check if the position is equal to NO_POSITION in the method assignFromView. The changed lines can be found here. I'm not sure about solution correctness, but it didn't break tests and crash gone for me.

@thagikura
Copy link
Contributor

@Bringoff thanks for you suggestion. Let me take a look at it.

@thagikura
Copy link
Contributor

I'm trying to reproduce the crash again on this branch, but no luck so far.
@Bringoff or is anyone able to share the code that can reproduce the quick?

The code looks good, but wanted to make sure if the fix is really appropriate.

@j-garin
Copy link

j-garin commented Jan 21, 2019

@thagikura still getting this FC

Issues and steps to reproduce

Update the items in androidx.recyclerview.widget.RecyclerView with androidx.recyclerview.widget.ListAdapter

Expected behavior

The list should handle multiple changes

Version of the flexbox library

1.1.0

Link to code

		val layoutManager = FlexboxLayoutManager(recyclerView.context)
				.apply {
					flexWrap = FlexWrap.WRAP
					flexDirection = FlexDirection.ROW
					justifyContent = JustifyContent.CENTER
					alignItems = AlignItems.CENTER
				}
		recyclerView.layoutManager = layoutManager
		recyclerView.adapter = adapter

happens when adapter.submitList(newList) gets called often. I have about 10 items in the list in my case, 1 item gets changed per call.

java.lang.IllegalStateException: The specified child already has a parent. You must call removeView() on the child's parent first.
        at android.view.ViewGroup.addViewInner(ViewGroup.java:4941)
        at android.view.ViewGroup.addView(ViewGroup.java:4772)
        at android.view.ViewGroup.addView(ViewGroup.java:4712)
        at androidx.recyclerview.widget.RecyclerView$5.addView(RecyclerView.java:851)
        at androidx.recyclerview.widget.ChildHelper.addView(ChildHelper.java:107)
        at androidx.recyclerview.widget.RecyclerView$LayoutManager.addViewInt(RecyclerView.java:8336)
        at androidx.recyclerview.widget.RecyclerView$LayoutManager.addView(RecyclerView.java:8294)
        at androidx.recyclerview.widget.RecyclerView$LayoutManager.addView(RecyclerView.java:8282)
        at com.google.android.flexbox.FlexboxLayoutManager.layoutFlexLineMainAxisHorizontal(FlexboxLayoutManager.java:1513)
        at com.google.android.flexbox.FlexboxLayoutManager.layoutFlexLine(FlexboxLayoutManager.java:1434)
        at com.google.android.flexbox.FlexboxLayoutManager.fill(FlexboxLayoutManager.java:1286)
        at com.google.android.flexbox.FlexboxLayoutManager.onLayoutChildren(FlexboxLayoutManager.java:754)
        at androidx.recyclerview.widget.RecyclerView.dispatchLayoutStep2(RecyclerView.java:3924)
        at androidx.recyclerview.widget.RecyclerView.dispatchLayout(RecyclerView.java:3641)

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

6 participants