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.

Sign up for GitHub

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

Jump to bottom

Occasional crash when expanding/collapsing group while updateAsync is running #286

Open
scorpeeon opened this issue Sep 12, 2019 · 6 comments
Open

Occasional crash when expanding/collapsing group while updateAsync is running #286

scorpeeon opened this issue Sep 12, 2019 · 6 comments
Labels
bug waiting for owners Waiting on an answer by project owners

Comments

@scorpeeon
Copy link

Describe the bug
When GroupAdapter.updateAsync() keeps getting called in the background and I click expand/collapse on an expandable item, occasionally I get a crash.

To Reproduce

I forked the project and created a branch with a modified example to reproduce the bug: https://github.com/scorpeeon/groupie/tree/updateasync-expandable-bug

Steps to reproduce the behavior:

When starting the example (non-databinding) the code will keep calling updateAsync to update the items. Meanwhile if I keep tapping the expand/collapse button on the expandable item, the code will eventually crash. (it might take a while for it to happen, tapping it rapidly tend to give faster results)

Expected behavior
Code should not crash when expanding items while using updateAsync.

Library version
2.5.1

Additional context

Example stacktraces here:

09-12 11:18:14.647 9694-9694/com.xwray.groupie.example E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.xwray.groupie.example, PID: 9694
    java.lang.IndexOutOfBoundsException: Inconsistency detected. Invalid view holder adapter positionViewHolder{1c8fea59 position=8 id=-1, oldPos=1, pLpos:1 scrap [attachedScrap] tmpDetached not recyclable(1) no parent} androidx.recyclerview.widget.RecyclerView{2633c910 VFED.... .F....ID 0,0-1200,1920 #7f090070 app:id/recyclerView}, adapter:com.xwray.groupie.GroupAdapter@1dafac09, layout:androidx.recyclerview.widget.GridLayoutManager@22d2e0e, context:com.xwray.groupie.example.MainActivity@2e895d57
        at androidx.recyclerview.widget.RecyclerView$Recycler.validateViewHolderForOffsetPosition(RecyclerView.java:5715)
        at androidx.recyclerview.widget.RecyclerView$Recycler.tryGetViewHolderForPositionByDeadline(RecyclerView.java:5898)
        at androidx.recyclerview.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:5858)
        at androidx.recyclerview.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:5854)
        at androidx.recyclerview.widget.LinearLayoutManager$LayoutState.next(LinearLayoutManager.java:2230)
        at androidx.recyclerview.widget.GridLayoutManager.layoutChunk(GridLayoutManager.java:557)
        at androidx.recyclerview.widget.LinearLayoutManager.fill(LinearLayoutManager.java:1517)
        at androidx.recyclerview.widget.LinearLayoutManager.onLayoutChildren(LinearLayoutManager.java:612)
        at androidx.recyclerview.widget.GridLayoutManager.onLayoutChildren(GridLayoutManager.java:171)
        at androidx.recyclerview.widget.RecyclerView.dispatchLayoutStep1(RecyclerView.java:3875)
        at androidx.recyclerview.widget.RecyclerView.dispatchLayout(RecyclerView.java:3639)
        at androidx.recyclerview.widget.RecyclerView.onLayout(RecyclerView.java:4194)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:579)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:514)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:579)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:514)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1703)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1557)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1466)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:579)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:514)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1703)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1557)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1466)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:579)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:514)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:2131)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1888)
        at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1106)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:5965)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:767)
        at android.view.Choreographer.doCallbacks(Choreographer.java:580)
        at android.view.Choreographer.doFrame(Choreographer.java:550)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:753)
        at android.os.Handler.handleCallback(Handler.java:739)
        at android.os.Handler.dispatchMessage(Handler.java:95)
    	at android

Another one:

09-12 11:10:58.997 9414-9414/com.xwray.groupie.example E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.xwray.groupie.example, PID: 9414
    java.lang.IndexOutOfBoundsException: Inconsistency detected. Invalid view holder adapter positionViewHolder{1de4d1b8 position=9 id=-1, oldPos=1, pLpos:1 scrap [attachedScrap] tmpDetached not recyclable(1) no parent} androidx.recyclerview.widget.RecyclerView{3c5d1749 VFED.... .F....ID 0,0-1200,1920 #7f090070 app:id/recyclerView}, adapter:com.xwray.groupie.GroupAdapter@1c31584e, layout:androidx.recyclerview.widget.GridLayoutManager@243e8d6f, context:com.xwray.groupie.example.MainActivity@2e895d57
        at androidx.recyclerview.widget.RecyclerView$Recycler.validateViewHolderForOffsetPosition(RecyclerView.java:5715)
        at androidx.recyclerview.widget.RecyclerView$Recycler.tryGetViewHolderForPositionByDeadline(RecyclerView.java:5898)
        at androidx.recyclerview.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:5858)
        at androidx.recyclerview.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:5854)
        at androidx.recyclerview.widget.LinearLayoutManager$LayoutState.next(LinearLayoutManager.java:2230)
        at androidx.recyclerview.widget.GridLayoutManager.layoutChunk(GridLayoutManager.java:557)
        at androidx.recyclerview.widget.LinearLayoutManager.fill(LinearLayoutManager.java:1517)
        at androidx.recyclerview.widget.LinearLayoutManager.onLayoutChildren(LinearLayoutManager.java:612)
        at androidx.recyclerview.widget.GridLayoutManager.onLayoutChildren(GridLayoutManager.java:171)
        at androidx.recyclerview.widget.RecyclerView.dispatchLayoutStep1(RecyclerView.java:3875)
        at androidx.recyclerview.widget.RecyclerView.dispatchLayout(RecyclerView.java:3639)
        at androidx.recyclerview.widget.RecyclerView.onLayout(RecyclerView.java:4194)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:579)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:514)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:579)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:514)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1703)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1557)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1466)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:579)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:514)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1703)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1557)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1466)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:579)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:514)
        at android.view.View.layout(View.java:15693)
        at android.view.ViewGroup.layout(ViewGroup.java:5038)
        at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:2131)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1888)
        at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1106)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:5965)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:767)
        at android.view.Choreographer.doCallbacks(Choreographer.java:580)
        at android.view.Choreographer.doFrame(Choreographer.java:550)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:753)
        at android.os.Handler.handleCallback(Handler.java:739)
        at android.os.Handler.dispatchMessage(Handler.java:95)
    	at androi
@scorpeeon scorpeeon added bug waiting for owners Waiting on an answer by project owners labels Sep 12, 2019
@khatv911
Copy link

I guess this will also happen when you write an adapter in a normal way and try to mess it up (calling notify...()) while it's dispatching its diffResult. Maybe there is a workaround like: disable all interactions with the Items when we begin updating and enable back via https://github.com/lisawray/groupie/blob/master/library/src/main/java/com/xwray/groupie/OnAsyncUpdateListener.java

@scorpeeon
Copy link
Author

@khatv911 Yeah, I imagine there could be other cases like that that can trigger this crash too.
I just saw an old comment by @lisawray mentioning that if updateAsync() is used, individual asynchronous updates for Items/Groups shouldn't be used as that might create a conflict:
#180
She even specifically mentions ExpandableGroup as something that could trigger this.
But these comment are only about individual asynchronous updates (which aren't even implemented for this specific reason), not synchronous ones that clicking on an ExpandableGroup should also trigger. As far as I can tell that should be supported, so I guess there must be a bug where in some rare occasions a race condition or edge case still occurs, causing this crash.

Yeah I guess a workaround like disabling all interactions while the update is running could work, though that might not be an ideal solution in itself as updating the list can be triggered in the background so users would be clueless why their actions are ignored sometimes.

@khatv911
Copy link

@scorpeeon i agree that it could be a race cond. though the name is async..., what is done in background is calculating DiffResult while dispatching that result is still in ui thread.

in my project, i already gave up expandablegroup and rebuild the whole list even when only one group needs expanding as a trade-off.

@scorpeeon
Copy link
Author

in my project, i already gave up expandablegroup and rebuild the whole list even when only one group needs expanding as a trade-off.

That's an interesting approach. I wonder how you handle expanding/collapsing items without ExpandableGroup, especially animations. Also if you don't need ExpandableGroup, I guess you might as well just use a ListAdapter..

@khatv911
Copy link

expandablegroup doesn’t involve any animations. it just purely call notifyItemRemoved and notifyItemAdded. the animations you see is from RecyclerView#ItemAnimator. So implement a effective DiffUtil will result the same.
2nd, groupie are not just for expandable. if you have multiple view type in a recyclerview you should go with Groupie or else you would find it quite tedious with ListAdapter.
the Item you write for one screen also can be used for other screen.
I also use ListAdapter now and then, but only if im sure there is only 1 or 2 view types in it.

@khatv911
Copy link

for collapsed/ expanded state, i had to store in a map. it actually the same approach when you handle checkboxes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug waiting for owners Waiting on an answer by project owners
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@scorpeeon @khatv911