idanatz / OneAdapter

A Viewholderless Adapter for RecyclerView, who supports builtin diffing, states (paging, empty...), events (clicking, swiping...), and more.
MIT License
470 stars 45 forks source link

Invalid view holder adapter position #13

Closed Minification closed 4 years ago

Minification commented 4 years ago

Hi, the library is really great due to its modular design. It really does help a lot.

Unfortunately I have encountered an issue. I will first show some code, but I do not have a small, executable example at the moment, I might add it later.

My code uses the OneAdapter as follows:

Within the fragment:

recyclerView = view.findViewById(R.id.ingredients_list);
recyclerView.setHasFixedSize(true);
recyclerView.setLayoutManager(new LinearLayoutManager(getActivity()));
oneAdapter = new OneAdapter(recyclerView);
oneAdapter.attachEmptinessModule(new PantryEmptinessModule());
oneAdapter.attachPagingModule(new PantryPagingModule());
oneAdapter.attachItemModule(new PantryItemModule());
prepopulateAdapter();

Within prepopulateAdapter (on main thread):

oneAdapter.add(recipes); //recipes is an ArrayList

Within PantryPagingModule:

@Override
public PagingModuleConfig provideModuleConfig() {
    return new PagingModuleConfig() {
        @Override
        public int withVisibleThreshold() {
            return 3;
        }
        @Override
        public int withLayoutResource() {
            return R.layout.load_more_item;
        }
    };
}

@Override
public void onLoadMore(int i) {
     //Recipes are obtained in the background...
     //on main thread
      oneAdapter.add(recipes); //recipes is an ArrayList
}

The item looks as follows:

@PrimaryKey
    public int id;

    @ColumnInfo(name = "name")
    public String name;

    @ColumnInfo(name = "amount")
    public int amount;

    @Override
    public boolean areContentTheSame(@NotNull Object o) {
        if (!(o instanceof Recipe)) {
            return false;
        }
        Ingredient other = (Recipe) o;
        boolean names = this.name.equals(other.name);
        boolean amount = this.amount == other.amount;
        return names && amount;
    }

    @Override
    public long getUniqueIdentifier() {
        return id;
    }

And the error message that sometimes comes when scrolling is:

ava.lang.IndexOutOfBoundsException: Inconsistency detected. Invalid view holder adapter positionViewHolder

Now, I have no idea where this error might come from. Maybe we can find a solution to this and fix it. If any more information is needed, I'll be happy to help. Thanks in advance.

idanatz commented 4 years ago

Hi, thanks for the kind words!

I've tried to re-create the crash with the example above in the sample project and could not replicate it.

What version of OneAdapter are you using? please be sure to be on the latest version. (the last version addressed this crash) If you are using the latest version, can you please share a simple as possible project that contains this bug? your help will be appreciated.

Minification commented 4 years ago

Hi @idanatz, I'm sorry I didn't provide the version - it was indeed 1.3.0, so not the latest version. I have now included the latest version (1.4.0), and the error seems to have gone. I was just wondering how I could get notified about new versions? Does starring help? Anyway, thank you for your time, you do very good work. I'll close the issue now

idanatz commented 4 years ago

Your welcome, glad it now works. In order to get notified about new versions, you can star and follow this repository (the 2 buttons next to fork)

meruiden commented 4 years ago

I still get this issue :( it appears to be super random and quite rare but it does happen.


Inconsistency detected. Invalid view holder adapter positionViewHolder{914c5d7 position=16 id=443383913, oldPos=1, pLpos:1 scrap [attachedScrap] tmpDetached not recyclable(1) no parent} 
androidx.recyclerview.widget.RecyclerView{7e62143 VFED..... ......ID 0,0-1440,2607 #7f0a011e 
app:id/recyclerView}, adapter:com.idanatz.oneadapter.internal.InternalAdapter@5b5952d, layout:androidx.recyclerview.widget.GridLayoutManager@f926a62, context:nl.wait.wait.activities.MainActivity@751eb1d

        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:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at androidx.swiperefreshlayout.widget.SwipeRefreshLayout.onLayout(SwipeRefreshLayout.java:625)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at androidx.constraintlayout.widget.ConstraintLayout.onLayout(ConstraintLayout.java:1915)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:270)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1829)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1673)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1582)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:270)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1829)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1673)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1582)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at androidx.drawerlayout.widget.DrawerLayout.onLayout(DrawerLayout.java:1231)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:270)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1829)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1673)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1582)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:270)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1829)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1673)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1582)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:332)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:270)
        at com.android.internal.policy.DecorView.onLayout(DecorView.java:1052)
        at android.view.View.layout(View.java:23706)
        at android.view.ViewGroup.layout(ViewGroup.java:6663)
        at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:3624)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3084)
        at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2148)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8830)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:996)
        at android.view.Choreographer.doCallbacks(Choreographer.java:794)
        at android.view.Choreographer.doFrame(Choreographer.java:729)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:981)
        at android.os.Handler.handleCallback(Handler.java:883)
        at android.os.Handler.dispatchMessage(Handler.java:100)
        at android.os.Looper.loop(Looper.java:237)
        at android.app.ActivityThread.main(ActivityThread.java:7770)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1047)
idanatz commented 4 years ago

Can you supply code samples that recreate your issue? even if it's not consistent it's a tricky one to find :(

meruiden commented 4 years ago

Its very random. I fill my list with data from an api on startup and 9/10 times everything is fine but sometimes it randomly crashes. were getting close to release and this is one of the most serious bugs we have :( I hope its figured out soon

meruiden commented 4 years ago

https://stackoverflow.com/questions/31603826/recyclerview-crashes-when-updating-on-top

could this be related perhaps?

idanatz commented 4 years ago

This is a very known issue with RecyclerView and i know its causes. The problem is that it's inconsistent and I can't find a case where the crash happens in my tests. Tell me, this 1/10 event happens when the adapter is empty and you set its data for the first time?

meruiden commented 4 years ago

No, it also sometimes happens when refreshing the data. it has not happened when adding new data with pagination once there already is data in the list, only when refreshing and on the first time

meruiden commented 4 years ago

I found a way to reproduce it for myself:

I accidentally fetch my list of data 2 times. so 2 times in a row I get the exact same data in different instances. If i call setItems with those 2 different instances of lists with the same data before onResume (hence why its so random, it depends on the api and internet speed) it crashes. so I think this has to do with the fact that diffutil thinks the data is the same and does nothing even though the instances of the models are different if you get what I mean.

Edit:

I this does not seem to be the case unfortunately. its weird. I am currently able to reproduce it though by putting a breakpoint before my setitems call and waiting a bit before pressing continue. this triggers it all the time

Edit:

I have 1 header (index 0) and 15 items. somehow when the crash happens the position of the header inside the list is 16 and not 0 like it should be. could this be an issue in DiffUtil or the way its used in OneAdapter?

meruiden commented 4 years ago

I think it has to do with the fast that in InternalAdapter 2 diffResult.dispatchUpdatesTo(listUpdateCallback) calls overlap because the backgroundExecutor does not wait for that operation to complete since it posts it on the ui thread

meruiden commented 4 years ago

I made a small project for you where its reproducible. I got it to trigger a few times on my samsung galaxy note 10+ but not on the pixel 2 api 28 emulator yet so try this on a physical (preferably samsung) device.

you can even test on a physical samsung note 10+ here: https://developer.samsung.com/remotetestlab/galaxy/rtlDeviceList.action#445

project: https://drive.google.com/file/d/1tvUyMHQ6UFsvPBZZxoEjXYby04SefcSZ/view?usp=sharing

if you cant manage to trigger it try this: comment at the while loop line 109 in MainActivity and put a breakpoint at line 111 setItems. as soon as it triggers, wait about 3-6 seconds and press continue. this method worked really well for triggering it for me

palVikas9 commented 4 years ago

@meruiden Please try recyclerView.setItemAnimator(null);

I was also having the same problem and this line solved the issue. I am not sure why this is happening but this helped me to get the problem solved. It is most probably occurring due the Diffable implementation of the model class.

meruiden commented 4 years ago

@palVikas9 thanks! this actually seems to work for some reason. I did like the animations tho so it would be nice if a fix could be introduced soon. But this will do for now!

palVikas9 commented 4 years ago

@meruiden Yeah, I am also hoping that a fix could be introduced soon as this would help us have nice animations in the application.

Glad that it worked for you too.

idanatz commented 4 years ago

Thanks guys for the replies and samples! Will try your suggestions ASAP

idanatz commented 4 years ago

Sadly, even with your project and guidance, I could not reproduce the crash. I've tested on number of real devices and emulators.

I've reexamined the whole background-UI threads handling code in the diffing mechanism and found an issue when an old diffing request that should have been canceled was still executed in some cases. This issue did not crash my tests but still, it could be the symptoms of the issues you are experiencing.

The fix will be published in the next version of the lib, in 1-2 days.

meruiden commented 4 years ago

Thanks! il try it out once you released it. hope thats it

idanatz commented 4 years ago

v1.5.1 is out closing (hoping for good this time)

Keickd commented 3 years ago

@meruiden Please try recyclerView.setItemAnimator(null);

I was also having the same problem and this line solved the issue. I am not sure why this is happening but this helped me to get the problem solved. It is most probably occurring due the Diffable implementation of the model class.

You saved my life, i spent several days looking for a solution and this line made my day man :)

idanatz commented 3 years ago

still happening on the latest version?

Keickd commented 3 years ago

still happening on the latest version?

I'm using the API 29 version

idanatz commented 3 years ago

still happening on the latest version?

I'm using the API 29 version

not android version, OneAdapter version... is it still happening on v2.1.0? (without the setItemAnimator(null) fix)

Keickd commented 3 years ago

I m not using OneAdapter version, in fact i m not sure you want mean, i create a custom adapter for my recycler view, and the problem was when i used the (adapterPosition)