Eliminate possible addon data race condition during update (#4619)

* Eliminate possible addon data race condition during update

* Fix pylint error

* Use Self type instead of quotes

supervisor/addons/__init__.py

@@ -290,6 +290,8 @@ async def update(
         # Update instance
         last_state: AddonState = addon.state
         old_image = addon.image
+        # Cache data to prevent races with other updates to global
+        store = store.clone()
         try:
             await addon.instance.update(store.version, store.image)
         except DockerError as err:
@@ -300,7 +302,9 @@ async def update(
 
         # Cleanup
         with suppress(DockerError):
-            await addon.instance.cleanup(old_image=old_image)
+            await addon.instance.cleanup(
+                old_image=old_image, image=store.image, version=store.version
+            )
 
         # Setup/Fix AppArmor profile
         await addon.install_apparmor()

supervisor/docker/interface.py

@@ -449,12 +449,17 @@ async def logs(self) -> bytes:
         return b""
 
     @Job(name="docker_interface_cleanup", limit=JobExecutionLimit.GROUP_WAIT)
-    def cleanup(self, old_image: str | None = None) -> Awaitable[None]:
+    def cleanup(
+        self,
+        old_image: str | None = None,
+        image: str | None = None,
+        version: AwesomeVersion | None = None,
+    ) -> Awaitable[None]:
         """Check if old version exists and cleanup."""
         return self.sys_run_in_executor(
             self.sys_docker.cleanup_old_images,
-            self.image,
-            self.version,
+            image or self.image,
+            version or self.version,
             {old_image} if old_image else None,
         )
 

supervisor/store/addon.py

@@ -1,22 +1,31 @@
 """Init file for Supervisor add-ons."""
+
+from copy import deepcopy
 import logging
+from typing import Self
 
 from ..addons.model import AddonModel, Data
+from ..coresys import CoreSys
 
 _LOGGER: logging.Logger = logging.getLogger(__name__)
 
 
 class AddonStore(AddonModel):
     """Hold data for add-on inside Supervisor."""
 
+    def __init__(self, coresys: CoreSys, slug: str, data: Data | None = None):
+        """Initialize object."""
+        super().__init__(coresys, slug)
+        self._data: Data | None = data
+
     def __repr__(self) -> str:
         """Return internal representation."""
         return f"<Store: {self.slug}>"
 
     @property
     def data(self) -> Data:
         """Return add-on data/config."""
-        return self.sys_store.data.addons[self.slug]
+        return self._data or self.sys_store.data.addons[self.slug]
 
     @property
     def is_installed(self) -> bool:
@@ -27,3 +36,7 @@ def is_installed(self) -> bool:
     def is_detached(self) -> bool:
         """Return True if add-on is detached."""
         return False
+
+    def clone(self) -> Self:
+        """Return a copy that includes data and does not use global store data."""
+        return type(self)(self.coresys, self.slug, deepcopy(self.data))

tests/addons/test_manager.py

@@ -15,6 +15,7 @@
 from supervisor.docker.addon import DockerAddon
 from supervisor.docker.const import ContainerState
 from supervisor.docker.interface import DockerInterface
+from supervisor.docker.manager import DockerAPI
 from supervisor.docker.monitor import DockerContainerStateEvent
 from supervisor.exceptions import (
     AddonConfigurationError,
@@ -23,6 +24,7 @@
     DockerNotFound,
 )
 from supervisor.plugins.dns import PluginDns
+from supervisor.store.repository import Repository
 from supervisor.utils import check_exception_chain
 from supervisor.utils.common import write_json_file
 
@@ -364,3 +366,41 @@ async def test_repository_file_error(
         write_json_file(repo_file, {"invalid": "bad"})
         await coresys.store.data.update()
         assert f"Repository parse error {repo_dir.as_posix()}" in caplog.text
+
+
+async def test_store_data_changes_during_update(
+    coresys: CoreSys, install_addon_ssh: Addon
+):
+    """Test store data changing for an addon during an update does not cause errors."""
+    event = asyncio.Event()
+    coresys.store.data.addons["local_ssh"]["image"] = "test_image"
+    coresys.store.data.addons["local_ssh"]["version"] = AwesomeVersion("1.1.1")
+
+    async def simulate_update():
+        async def mock_update(_, version, image, *args, **kwargs):
+            assert version == AwesomeVersion("1.1.1")
+            assert image == "test_image"
+            await event.wait()
+
+        with patch.object(DockerAddon, "update", new=mock_update), patch.object(
+            DockerAPI, "cleanup_old_images"
+        ) as cleanup:
+            await coresys.addons.update("local_ssh")
+            cleanup.assert_called_once_with(
+                "test_image", AwesomeVersion("1.1.1"), {"local/amd64-addon-ssh"}
+            )
+
+    update_task = coresys.create_task(simulate_update())
+    await asyncio.sleep(0)
+
+    with patch.object(Repository, "update"):
+        await coresys.store.reload()
+
+    assert "image" not in coresys.store.data.addons["local_ssh"]
+    assert coresys.store.data.addons["local_ssh"]["version"] == AwesomeVersion("9.2.1")
+
+    event.set()
+    await update_task
+
+    assert install_addon_ssh.image == "test_image"
+    assert install_addon_ssh.version == AwesomeVersion("1.1.1")