Race condition in snaplist causing list of snaps to be (partially) empty

Bug #1989193 reported by Olivier Gayot
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
subiquity (Ubuntu)
Fix Released
Medium
Olivier Gayot

Bug Description

Because of a race condition, it is possible that the screen supposed to show the list of snaps shows an empty list of snaps (or partially empty).

The race condition is triggered when a network changed event (i.e., SNAPD_NETWORK_CHANGE) occurs while we are fetching the list of snaps.

The code vulnerable to the race condition is in this function from subiquity/server/controllers/snaplist.py:
```
    async def GET(self, wait: bool = False) -> SnapListResponse:
        if self.loader.failed or not self.app.base_model.network.has_network:
            await self.configured()
            return SnapListResponse(status=SnapCheckState.FAILED)
        if not self.loader.snap_list_fetched and not wait:
            return SnapListResponse(status=SnapCheckState.LOADING)
        await self.loader.get_snap_list_task().wait()
        if self.loader.failed or not self.app.base_model.network.has_network:
            await self.configured()
            return SnapListResponse(status=SnapCheckState.FAILED)
        return SnapListResponse(
            status=SnapCheckState.DONE,
            snaps=self.model.get_snap_list(),
            selections=self.model.selections)
```

And more specifically between the two following instructions:
```
        await self.loader.get_snap_list_task().wait()
        if self.loader.failed or not self.app.base_model.network.has_network:
```

When a SNAPD_NETWORK_CHANGE event is fired, the following happens:
 * the task that was returned by self.loader.get_snap_list_task() gets cancelled silently (without setting failed=True on the loader).
 * a new loader is created and replaces the old one: `self.loader = self._make_loader()`

Therefore, upon reaching the following instruction
```
        if self.loader.failed or not self.app.base_model.network.has_network:
```

, self.loader is now a new instance of SnapdSnapInfoLoader. That new instance is initialized with failed=False so the code does not go through the if body.

The first thing that comes to mind to fix this issue would be to do something like:

```
        loader = self.loader
        await loader.get_snap_list_task().wait()
        if loader.failed or not self.app.base_model.network.has_network:
```
but ideally, we would not consider this event an error and we would wait for the task of the new loader to finish.

Revision history for this message
Olivier Gayot (ogayot) wrote :
Changed in subiquity (Ubuntu):
status: New → In Progress
assignee: nobody → Olivier Gayot (ogayot)
Changed in subiquity (Ubuntu):
importance: Undecided → Medium
Olivier Gayot (ogayot)
Changed in subiquity (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
Dan Bungert (dbungert) wrote :

We believe a fix for this can be found in Subiquity 22.10.1. On
install you will be offered to update to the new version of the
installer if network is available, or you can perform a manual update
by running the follwing in a terminal:
sudo snap refresh subiquity

Changed in subiquity (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.