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

fix: #1786 - safe write access to barcode lists #1792

Merged
merged 1 commit into from
May 9, 2022

Conversation

monsieurtanuki
Copy link
Contributor

Impacted file:

  • dao_product_list.dart: safe write access to barcode lists

What

  • There was a very hard to believe bug, with a concurrency exception (typically, a list being parsed while items are removed/added).
  • This sounded crazy, because I expected each hive/get call to create a new object.
  • With hive this is not the case, as all data are loaded at init, and we reuse the same objects.
  • That explains the probability of concurrency exception: there are methods that list all items, other methods that add/remove items, and all of them may use the same object (loaded at init) at the same time.
  • The solution was to be careful each time we add/remove items, with a safe copy of the list we intend to edit.
  • I could not reproduce the bug btw.

Fixes bug(s)

Fixes: #1786
Fixes SMOOTHIE-V2

Impacted file:
* `dao_product_list.dart`: safe write access to barcode lists

Fixes SMOOTHIE-V2
@monsieurtanuki monsieurtanuki requested a review from a team as a code owner May 9, 2022 11:57
Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a really unlikely case. I can't say that this solves the problem but doesn't seem to add much complexity

@monsieurtanuki
Copy link
Contributor Author

Thank you @M123-dev for your review!
I can't say either if it is 100% relevant, but I could not reproduce the bug (that did not happen often anyway) and I took me one hour to understand how a list that was only read could have a concurrency issue (answer: hive caches everything).

@monsieurtanuki monsieurtanuki merged commit 6cd8008 into openfoodfacts:develop May 9, 2022
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

Successfully merging this pull request may close these issues.

ConcurrentModificationError: Concurrent modification during iteration: Instance(length:22) of '_GrowableList'.
2 participants