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

Eliminate possible addon data race condition during update #4619

Merged
merged 3 commits into from
Oct 11, 2023

Conversation

mdegat01
Copy link
Contributor

Proposed change

Currently addon update has a possible race condition with the global addon data. This can be triggered as follows:

  1. An addon update is initiated
  2. While waiting for the new image to download, store data is updated (possibly due to scheduled task)
  3. The config of the addon being updated has changed. Specifically if a new version came out or the image was changed

In this situation Supervisor ends up pulling one image but telling the addon a completely different image is in use. And then promptly removing the image it just pulled as it believes it is stale.

The unit test reproduces this race condition by simulating with asyncio events prior to the code change. It is not reproducible afterwards. I believe this change to be the solution to #4610 . Although we can't be certain as we have no logs or steps to reproduce clearly showing what is occurring on these systems. Still it seems likely as the steps I outlined above would result in that exact error.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality to the supervisor)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast supervisor tests)
  • Tests have been added to verify that the new code works.

If API endpoints of add-on configuration are added/changed:

@mdegat01 mdegat01 marked this pull request as ready for review October 11, 2023 00:35
@mdegat01 mdegat01 requested a review from pvizeli October 11, 2023 00:35
@mdegat01 mdegat01 added the bugfix A bug fix label Oct 11, 2023
@mdegat01 mdegat01 requested a review from agners October 11, 2023 00:35
supervisor/store/addon.py Outdated Show resolved Hide resolved
@mdegat01 mdegat01 force-pushed the fix-addon-start-no-image branch from 08c6749 to 9f9537b Compare October 11, 2023 15:27
@mdegat01 mdegat01 merged commit 1376a38 into main Oct 11, 2023
20 checks passed
@mdegat01 mdegat01 deleted the fix-addon-start-no-image branch October 11, 2023 16:22
@github-actions github-actions bot locked and limited conversation to collaborators Oct 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image homeassistant/amd64-addon-zwave_js:0.1.96 does not exist for addon_core_zwave_js
2 participants