[updates] Displays a spinner indefinitely when an update download was started externally

[Iain Lane]

These packages are currently in the CI train PPA <https://launchpad.net/~ci-train-ppa-service/+archive/landing-010/>. Upgrade to these first.

1. Make sure you're on auto download of updates (wi-fi) and that there's a system image update available to you
2. Open system-settings
3. Wait for "Updates available" to show, and click it
4. "Checking for updates" and an activity indicator appear
5. Wait. Don't let the screen blank (bug #1259326 - not sure this is actually still a problem but better safe than sorry). The update is downloading in the background at this point.
6. When the update has finished downloading, you'll see the standard popover inviting you to install or cancel.

What should have happened is: instead of or shortly after step 4, we should have seen the status of the in-progress download, just as if we'd started it from the update panel.

[Iain Lane (laney)]

I just did some debugging, and I now believe this is a problem in system-image-dbus.

The u-s-s main screen component and panel both call CheckForUpdate on s-i's D-Bus interface. This is supposed (per https://wiki.ubuntu.com/ImageBasedUpgrades/Client#Methods --- "In all cases, an UpdateAvailableStatus signal will be issued once an update candidate is determined to be available") to result in an UpdateAvailableStatus being issued. It's this signal that the UI relies on to update itself in order to indicate that an update in progress.

For the second call (once you enter the updates screen), this signal is never actually emitted. On the console the following is printed

  [systemimage] Feb 25 10:17:40 2014 (8412) test and acquire checking lock
  [systemimage] Feb 25 10:17:40 2014 (8412) checking lock not acquired

In testing mode (--testing=update-auto-success), this does work properly.

Now, it's possible that system-settings could pass the same updateManager model from the entry (main screen) component into the page so that we don't need to repeat the check and therefore can avoid the problem, but this seems to me to also be a s-i bug too.

[Iain Lane (laney)]

I'll investigate the u-s-s part so we can hopefully get moving on this quicker.

[Iain Lane (laney)]

Mmm, I just realised that we can't keep the object if the user goes About > Check for updates. So the s-i fix will be needed for this case.

[Barry Warsaw (barry)]

I think we talked about this - there isn't a second UAS signal for the second CFU method call. Because an update check is already in progress when the second CFU is made, the second one returns immediately. The second call *can't* send a UAS because it doesn't yet know whether an update is available (that check is already in progress). Because CFU is asynchronous, there's nothing more it can really do.

But you will still get an UAS signal when the previously in-progress check is complete, and you will still get UpdateProgress signals once the download is in progress.

Shouldn't both the main panel and the Update panel both be listening on the D-Bus for the UAS signal? Why do you need two UAS signals?

You mention the "checking lock not acquired" message, but that's just a debugging log indicating that the lock was not acquired after the second CFU call. That makes sense - the first check is still running in the background. When that first check completes, you'll get the expected UAS signal.

[Iain Lane (laney)]

I don't remember talking about this before, but maybe I just forgot. :)

Anyway. The UAS signal contains all of the details about the update that the UI needs to display itself.

In system-settings, the two checks are quite separate. Think of them as two different clients if possible. In fact, it wouldn't even be possible to get this bug if you hadn't received at least one UAS, because it is this signal that the UI looks at to decide whether to show you the menu item that takes you to the updates page in the first place.

It's like this

 → checkForUpdate
 ← UAS
 ← UpdateProgress …
 (user clicks on 'updates available')
 → checkForUpdate
 ← silence

The UI at this point could pick up the UpdateProgress signals that are happening, but it can't give you any of the normal details about the update that is currently being downloaded.

I'm not asking for a 1-1 mapping between checkForUpdate calls and UpdateAvailableStatus emissions, but it seems that there is a bug where UAS is never emitted while a download is in progress (in 'real' only, under testing it does indeed work for me). Actually, I forgot to check if it's emitted after the download is finished too. Can do that if it's important.

The message I pasted in the earlier comment was printed as a result of the second call I made.

Is *this* expectation unreasonable? I'm going to attach a dbus log that shows the signals that s-i-dbus emitted during my test run. Somewhere in the middle of the downloading phase, after the UAS, is where I tried to do some more checkForUpdate calls.

[Barry Warsaw (barry)]

Okay, I think I see what you're getting at. What you're observing is that when auto-downloads are enabled, you will get the UAS signal first, but the checking lock won't be released until after the download completes, at which time you'll also get a Update{Downloaded,Failed} signal.

I did it this way because I still observed download failures when the checking lock was released during an automatic download. Now, it's possible that the other fixes I and Manuel implemented would make those download failures go away (i.e. because of fixes to u-d-m, or workarounds in s-i), but we'd have to be very careful to test that vigorously before I'd be confident in releasing the check lock earlier.

I can think of a few other ways to handle this, short of mucking with the lock release:

In the initial check that s-s performs, turn off automatic downloads. Then you'll get the UAS signal and lock release as quickly as possible. Then re-enable auto-downloads and issue the second CFU. The actual check should return quite quickly, since we can use the cached results (we'll still have to grab the blacklist, but that shouldn't be much of a delay).

Ah, I just thought of a possible middle ground: if CFU is called again after the actual checking phase is complete, but before the auto-download is complete, then we *do* have some available status and could send the UAS signal at that point.

[Manuel de la Peña (mandel)]

On Tue, Feb 25, 2014 at 4:39 PM, Barry Warsaw <email address hidden>wrote:

> Okay, I think I see what you're getting at. What you're observing is
> that when auto-downloads are enabled, you will get the UAS signal first,
> but the checking lock won't be released until after the download
> completes, at which time you'll also get a Update{Downloaded,Failed}
> signal.
>
> I did it this way because I still observed download failures when the
> checking lock was released during an automatic download. Now, it's
> possible that the other fixes I and Manuel implemented would make those
> download failures go away (i.e. because of fixes to u-d-m, or
> workarounds in s-i), but we'd have to be very careful to test that
> vigorously before I'd be confident in releasing the check lock earlier.
>
>
Would be interesting to see what happens if you do not do that and if the
downloads occur they step on the present file... of course that means
reverting changes for testing, but if it fixes the issue...

Could an other solution tell the ui to connect to udm and check if there is
a download in process for the updates??? that way, before performing any
checkForUpdate we can ensure that the download is not being done. Am I just
saying something stupid?

I can think of a few other ways to handle this, short of mucking with
> the lock release:
>
> In the initial check that s-s performs, turn off automatic downloads.
> Then you'll get the UAS signal and lock release as quickly as possible.
> Then re-enable auto-downloads and issue the second CFU. The actual
> check should return quite quickly, since we can use the cached results
> (we'll still have to grab the blacklist, but that shouldn't be much of a
> delay).
>
> Ah, I just thought of a possible middle ground: if CFU is called again
> after the actual checking phase is complete, but before the auto-
> download is complete, then we *do* have some available status and could
> send the UAS signal at that point.
>
> --
> You received this bug notification because you are a member of Ubuntu
> System Image team, which is subscribed to Ubuntu system image.
> https://bugs.launchpad.net/bugs/1284217
>
> Title:
> [updates] Displays a spinner indefinitely when an update download was
> started externally
>
> Status in Ubuntu system image (server/client/updater):
> New
> Status in "system-image" package in Ubuntu:
> New
> Status in "ubuntu-system-settings" package in Ubuntu:
> Triaged
>
> Bug description:
> These packages are currently in the CI train PPA
> <https://launchpad.net/~ci-train-ppa-service/+archive/landing-010/>.
> Upgrade to these first.
>
> 1. Make sure you're on auto download of updates (wi-fi) and that there's
> a system image update available to you
> 2. Open system-settings
> 3. Wait for "Updates available" to show, and click it
> 4. "Checking for updates" and an activity indicator appear
> 5. Wait. Don't let the screen blank (bug #1259326 - not sure this is
> actually still a problem but better safe than sorry). The update is
> downloading in the background at this point.
> 6. When the update has finished downloading, you'll see the standard
> popover inviting you to ...

[Barry Warsaw (barry)]

On Feb 25, 2014, at 03:59 PM, Manuel de la Peña wrote:

>> I did it this way because I still observed download failures when the
>> checking lock was released during an automatic download. Now, it's
>> possible that the other fixes I and Manuel implemented would make those
>> download failures go away (i.e. because of fixes to u-d-m, or
>> workarounds in s-i), but we'd have to be very careful to test that
>> vigorously before I'd be confident in releasing the check lock earlier.
>>
>Would be interesting to see what happens if you do not do that and if the
>downloads occur they step on the present file... of course that means
>reverting changes for testing, but if it fixes the issue...

I'd have to think very carefully about the concurrency issues involved, which
is why I'm taking the conservative approach of disallowing multiple CFU while
auto-download is in progress. You're right that I'm most worried about
clobbering existing files, since some of the downloads occur to generic local
file names (e.g. keyrings.tar.xz before it's moved and unpacked).

>Could an other solution tell the ui to connect to udm and check if there is
>a download in process for the updates??? that way, before performing any
>checkForUpdate we can ensure that the download is not being done. Am I just
>saying something stupid?

Not stupid at all! I was going to suggest something like that before I had my
previous revelation. :) That would however entail an API change, but it could
probably be done in a backward compatible way. Since we're past feature
freeze though, I didn't want to have to force both s-i and s-s to make that
change.

Let's see if the current patch will do the trick.

[Iain Lane (laney)]

Thanks Barry. That seems to work for me!

[Launchpad Janitor (janitor)]

This bug was fixed in the package system-image - 2.1-0ubuntu4

---------------
system-image (2.1-0ubuntu4) trusty; urgency=medium

  [ Stéphane Graber ]
  * New upstream release.
  * Set X-Auto-Uploader to no-rewrite-version
  * Set Vcs-Bzr to the new target branch

  [ Barry Warsaw ]
  * New upstream release.
    - LP: #1279056 - Internal improvements to SignatureError for
      better debugging.
    - LP: #1277589 - Better protection against race conditions.
    - LP: #1260768 - Return empty string from ApplyUpdate D-Bus method.
    - LP: #1284217 - Send UpdateAvailableStatus during auto-downloading
      from a previous CheckForUpdate, if cached status is available.
    - Request ubuntu-download-manager to download to a temporary location,
      with atomic rename.
    - More detailed logging.
    - Fixed D-Bus error logging.
    - Added -L flag to nose2 tests for explicitly setting log file path.
    - Added SYSTEMIMAGE_DBUS_DAEMON_HUP_SLEEP_SECONDS environment variable
      which can be used to give virtualized buildds a fighting chance.
  * d/patches/01_send_ack_on_applyupdate.diff: Removed; applied upstream.
  * d/patches/lp1284217.patch: Added (see above).
  * d/control:
    - Bump Standards-Version to 3.9.5 with no other changes necessary.
    - Add python3-psutil as Depends to system-image-dev.
  * d/rules: Set SYSTEMIMAGE_DBUS_DAEMON_HUP_SLEEP_SECONDS to 1 to deal with
    buildd dbus-daemon SIGHUP timing issues.
 -- Ubuntu daily release <email address hidden> Tue, 25 Feb 2014 17:48:27 +0000