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

Double notifyDataSetChanged may cause crash #17

Closed
Gloix opened this issue Mar 23, 2018 · 4 comments
Closed

Double notifyDataSetChanged may cause crash #17

Gloix opened this issue Mar 23, 2018 · 4 comments

Comments

@Gloix
Copy link

Gloix commented Mar 23, 2018

I don't know if it's a bad use of the library or not, but when performing the following sequence of actions, bad things can occur.

  1. Create a list with one item and create an adapter with it
  2. Call notifyDataSetChanged(). After that, getItemCount() gets called, returning 1 as result.
  3. Immediately, and before the adapter is able to refresh its annotations, remove the single item from the list
  4. Call notifyDataSetChanged(). After that, getItemCount() gets called, returning 0 as result.
  5. In the next view refresh, the adapter calls onCreateAnnotation(...) with position=0, leading to a crash since there are no items in the list.

Apparently, the adapter remembers the first amount of annotations to be added to the map, but the second call to notifyDataSetChanged() does not get taken into account.

@Gloix Gloix changed the title Double notifyDataSetChanged may cause crash. Double notifyDataSetChanged may cause crash Mar 23, 2018
@Gloix
Copy link
Author

Gloix commented Mar 23, 2018

Maybe an updates.clear() inside notifyDataSetChanged() could solve it(?). Unfortunately I cannot quick-fix in my project it since it's a private member 😕.

@josh-burton
Copy link
Collaborator

Would you be able to create a sample project that demonstrates the crash?

@Gloix
Copy link
Author

Gloix commented Mar 23, 2018

Sure, here it is
https://github.com/Gloix/MapMeBug
Remember to use a valid Google Maps Key to test it. Also, note that I don't even have to modify the list of adapted items to trigger the error. I just need to call notifyDataSetChanged() twice.

@josh-burton
Copy link
Collaborator

Great, thanks for this!

It seems like the fix will be quite simple although it will require a bit of testing.

josh-burton added a commit to josh-burton/MapMe that referenced this issue Mar 24, 2018
Fixes a crash caused by pending updates attempting to operate on a annotation list that is no longer there.

Fixes TradeMe#17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants