idanatz / OneAdapter

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

CalledFromWrongThreadException in 2.1.0 #38

Closed vellrya closed 3 years ago

vellrya commented 3 years ago

Hello, thanks for update.

Seems like calling oneAdapter.setItems(...) from background thread no longer supported in 2.1.0, but works fine in 2.0.2. Is it expected behaviour?

Code:

lifecycleScope.launch(Dispatchers.Default) {
    val items = ...
    oneAdapter.setItems(items)
}

Result:

2021-04-05 10:48:06.193 29570-29650/a.b.c E/AndroidRuntime: FATAL EXCEPTION: DefaultDispatcher-worker-2
    Process: a.b.c, PID: 29570
    android.view.ViewRootImpl$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
        at android.view.ViewRootImpl.checkThread(ViewRootImpl.java:8648)
        at android.view.ViewRootImpl.requestLayout(ViewRootImpl.java:1533)
        ...

Seems like this can easily be fixed with: withContext(Dispatchers.Main) { oneAdapter.setItems(items) }, but probably this should be done on the library side for backwards compatibility?

idanatz commented 3 years ago

Thanks for the reports, will be fixed and covered in test in the next release.

vellrya commented 3 years ago

Also I checked for https://github.com/ironSource/OneAdapter/issues/37 issue. There is no crash now in 2.1.0, but it is possible to achieve that after a certain amount of quick removal of all items and adding them back, the setItems method just stops working - the adapter remains empty, even if you pass some objects there.

Upd: If we call layoutManager.scrollToPositionWithOffset(position, 0) on broken empty adapter, then elements appear - probably this information can help in debug problem.

idanatz commented 3 years ago

I've tried to simulate the behavior you mention:

can you share the simplest project you can that this behavior is reproducible?

vellrya commented 3 years ago

Spent almost an hour to be able to strongly reproduce bug, but I did it)) I send you a video for reproducing steps and sample project later.

vellrya commented 3 years ago

https://github.com/vellrya/OneAdapter_Issue

Watch reproduce.mp4 video, bug appeared at ~0:40 Seems like this happened after we call setItems on empty adapter (when search substring reduced) after fast changing it's data

idanatz commented 3 years ago

Thanks for the help. I'm still trying to figure this behavior out - if it's my library or the extended Android adapter bug, and how to work around it.

vellrya commented 3 years ago

Now I switched to ListAdapter for this particular RecyclerView and cannot reproduce this error more. But of course I had to write a lot of boilerplate code and there is no support for pagination and a lot of other features of your library) Also, it seemed to me that with ListAdapter the work of RecyclerView became smoother, could this be?

idanatz commented 3 years ago

Honestly, ListAdapter is implemented by using AsyncListDiffer, which OneAdapter uses as well. The implementations are basically identical, but as you said with fewer features, hence maybe there is a slim benefit to the "Valina" version.

This is why this bug is so weird and even more, if it does not reproduce on ListAdapter. The entire state of the adapter is correct and keeps updating but the recycler seems disconnected from it.

Will keep investigating...

idanatz commented 3 years ago

The original issue has been fixed on v2.1.1