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

IOOB exception when using DiffUtil on separate thread #984

Open
3 tasks done
boguszpawlowski opened this issue Mar 4, 2021 · 5 comments
Open
3 tasks done

IOOB exception when using DiffUtil on separate thread #984

boguszpawlowski opened this issue Mar 4, 2021 · 5 comments
Assignees

Comments

@boguszpawlowski
Copy link

About this issue

When using FastAdapterDiffUtil.calculateDiff() on any thread different from Main, there is a chance of IndexOutOfBoundsException being thrown. It is reproducible on modified DiffUtilActivity from FastAdapter's sample:
https://github.com/boguszpawlowski/FastAdapter/blob/diffutil_bug/app/src/main/java/com/mikepenz/fastadapter/app/DiffUtilActivity.kt
Steps to reproduce:

  • Open DiffUtil Screen
  • Start scrolling
  • After hitting a specific time window, a crash will happen:
E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.mikepenz.fastadapter.app, PID: 2075
    java.lang.IndexOutOfBoundsException: Index: 26, Size: 19
        at java.util.ArrayList.get(ArrayList.java:437)
        at com.mikepenz.fastadapter.utils.DefaultItemListImpl.get(DefaultItemListImpl.kt:23)
        at com.mikepenz.fastadapter.adapters.ModelAdapter.getAdapterItem(ModelAdapter.kt:168)
        at com.mikepenz.fastadapter.FastAdapter.getItem(FastAdapter.kt:507)
        at com.mikepenz.fastadapter.FastAdapter.getItemViewType(FastAdapter.kt:581)
        at androidx.recyclerview.widget.RecyclerView$Recycler.validateViewHolderForOffsetPosition(RecyclerView.java:5978)
        at androidx.recyclerview.widget.RecyclerView$Recycler.tryGetViewHolderForPositionByDeadline(RecyclerView.java:6158)
        at androidx.recyclerview.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:6118)
        at androidx.recyclerview.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:6114)
        at androidx.recyclerview.widget.LinearLayoutManager$LayoutState.next(LinearLayoutManager.java:2303)
        at androidx.recyclerview.widget.LinearLayoutManager.layoutChunk(LinearLayoutManager.java:1627)
        at androidx.recyclerview.widget.LinearLayoutManager.fill(LinearLayoutManager.java:1587)
        at androidx.recyclerview.widget.LinearLayoutManager.scrollBy(LinearLayoutManager.java:1391)
        at androidx.recyclerview.widget.LinearLayoutManager.scrollVerticallyBy(LinearLayoutManager.java:1128)
        at androidx.recyclerview.widget.RecyclerView.scrollStep(RecyclerView.java:1841)
        at androidx.recyclerview.widget.RecyclerView$ViewFlinger.run(RecyclerView.java:5302)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1031)
        at android.view.Choreographer.doCallbacks(Choreographer.java:854)
        at android.view.Choreographer.doFrame(Choreographer.java:785)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1016)
        at android.os.Handler.handleCallback(Handler.java:914)
        at android.os.Handler.dispatchMessage(Handler.java:100)
        at android.os.Looper.loop(Looper.java:224)
        at android.app.ActivityThread.main(ActivityThread.java:7560)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:539)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:950)

Observations:

  • Error only occurs when diff calculation is made on thread different from Main
  • Error only occurs when you are interacting with a screen in the same time (e.g. scrolling) and large number of items is removed.

My conclusion: If diff calculation is happening at the same time as some changes on the screen (user interaction, change in view hierarchy), and is done concurrently, due to item list modified and read on different threads, crash can happen.

Details

  • Used library version - 5.3.5
  • Used support library version - N/A
  • Used gradle build tools version - 4.1.2
  • Used tooling / Android Studio version - Arctic Fox 2021 Canary 4
  • Other used libraries, potential conflicting libraries - N/A

Checklist

Similar/Connected issues:
#772 - potential solution
#959 - key difference is that I'm NOT using Objects.hash for creating id, they are 100% stable and unique.

@mikepenz mikepenz self-assigned this Mar 5, 2021
@mikepenz
Copy link
Owner

Thank you so much for writing up the separate ticket @boguszpawlowski

This problem is a very interesting one in it's base as it results due to inconsistencies in different threads, which the RV absolutely does not like.

Any help to debug this or open a PR with a solution is apprecaited

@AhmedGamal92
Copy link

Hello there

So we've been facing this crash also with DIffUtil and looks like the issue in the postCalculate method happening at the end of calculateDiff as it changes the list of items in the adapter, and if that's happening in another thread that leads to the inconsistency with the RV and then the crash
Then maybe moving the postCalculate part to happen with the setting of the result can solve the issue.

@mikepenz happy to open a PR with that approach but that will change the exposed methods in the FastAdapterDIffUtil class as the set methods doesn't have the new list of items

@mikepenz
Copy link
Owner

@AhmedGamal92 that sounds indeed like a good point. RV's don't appreciate it if changes are done while other changes get posted.

Is it a possibility to only do changes via the DiffUtil and prevent any other changes beyond that? (not 100% sure, if that's related to the FastAdapter code, or if that's just how it's intended to be?)

Based on your proposal, have you experimented doing the change to move the postCalculate method?
Not 100% sure anymore, but I feel there were problems a few years back when the diff util support was first added.

@AhmedGamal92
Copy link

Is it a possibility to only do changes via the DiffUtil and prevent any other changes beyond that?

Yes that was the idea, the place that does these extra changes is the postCalculate method, without it it will be only calculating the diff.

Based on your proposal, have you experimented doing the change to move the postCalculate method?

Yes I did that and I see the exception is no longer happening, also tried in the activity mentioned in the issue and can't reproduce after.

@joaocruz04
Copy link

Hello! Any updates on this issue?
@mikepenz did you try @AhmedGamal92 idea?

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

4 participants