FastAdapter icon indicating copy to clipboard operation
FastAdapter copied to clipboard

IOOB exception when using DiffUtil on separate thread

Open boguszpawlowski opened this issue 4 years ago • 5 comments

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: https://github.com/mikepenz/FastAdapter/issues/772 - potential solution https://github.com/mikepenz/FastAdapter/issues/959 - key difference is that I'm NOT using Objects.hash for creating id, they are 100% stable and unique.

boguszpawlowski avatar Mar 04 '21 10:03 boguszpawlowski

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

mikepenz avatar Mar 26 '21 10:03 mikepenz

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

AhmedGamal92 avatar Aug 13 '21 08:08 AhmedGamal92

@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.

mikepenz avatar Aug 14 '21 15:08 mikepenz

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.

AhmedGamal92 avatar Aug 17 '21 08:08 AhmedGamal92

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

joaocruz04 avatar May 13 '22 12:05 joaocruz04