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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

MvxRecycler view must not post NotifyDataSetChanged on MainLooper #3295

Closed
1 of 7 tasks
softlion opened this issue Feb 22, 2019 · 3 comments 路 Fixed by #3366
Closed
1 of 7 tasks

MvxRecycler view must not post NotifyDataSetChanged on MainLooper #3295

softlion opened this issue Feb 22, 2019 · 3 comments 路 Fixed by #3366
Labels
p/android-support Android Support Packages platform t/bug Bug type
Milestone

Comments

@softlion
Copy link
Contributor

softlion commented Feb 22, 2019

馃敊 Regression

Introduced by the fix for #2144

There may be a delay between the change of the source collection and the calls to NotifyItemXXX methods of the adapter, resulting in desynchronizations of the recycler's view and its datasource, thus generating random crashes when the recyclerview try to load a removed item.

Old (and correct) behavior

When an ObservableCollection is structurally updated in a viewmodel, and this collection is bound to the ItemsSource of a RecyclerView's adapter, the adapter subscribes to INotifyCollectionChanged and dispatch these events to NotifyDataSetChanged().

If the viewmodel's collection is changed on a worker thread, the app will crash. But there will be no delay between the change of the collection and the recycler's view notifications.

Current behavior

To prevent crashes when a viewmodel's collection is changed on a worker thread, the adapter posts the change on the MainLooper. This introduces the delay explained at the start of this bug report and leads to random crashes.

            if (Looper.MainLooper == Looper.MyLooper())
            {
                NotifyDataSetChanged(e);
            }
            else
            {
                var h = new Handler(Looper.MainLooper);
                h.Post(() => NotifyDataSetChanged(e));
            }

Reproduction steps

  1. Connect an ObservableCollection to an MvxRecyclerView, add lots of items.
  2. Remove an item every second in your viewmodel from a worker thread (timer)
  3. scroll the list in the app, it will crash at some point.

Configuration

Version: 6.x

Platform:

  • 馃摫 iOS
  • 馃 Android
  • 馃弫 WPF
  • 馃寧 UWP
  • 馃崕 MacOS
  • 馃摵 tvOS
  • 馃悞 Xamarin.Forms
@nmilcoff
Copy link
Contributor

Thanks for reporting. Can you please submit a PR fixing your feature request (#2144) and this "regression" issue?

@softlion

This comment was marked as abuse.

@softlion

This comment was marked as abuse.

@softlion softlion changed the title MvxRecycler view should not post NotifyDataSetChanged on MainLooper MvxRecycler view must not post NotifyDataSetChanged on MainLooper Mar 2, 2019
@Cheesebaron Cheesebaron added t/bug Bug type p/android-support Android Support Packages platform labels Mar 21, 2019
@martijn00 martijn00 added this to the 6.3.0 milestone May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p/android-support Android Support Packages platform t/bug Bug type
Development

Successfully merging a pull request may close this issue.

4 participants