Thanks for the background Diego.  Some thoughts inline.

On Feb 14, 2014, at 02:23 PM, Diego Sarmentero wrote:

>- The last situation is when automatic updates is activated, this seems
>to be another process (I've not worked with that code), that triggers
>another checkForUpdates when System Settings is opened in order to
>decide if it needs to start upgrading the system.

Remember that the default setting for system-image is to automatically
download the updates after a check-for-updates finds one available.  So even
the first check call will trigger the downloads.  You may or may not want to
do that based on whether you'd like the data files to be immediately
available.  That's a ux decision, but do be aware of that.  Also note that the
downloaded files are cached so a subsequent check will verify the validity of
the cached files, but can avoid costly downloads if they're all still valid.

You can of course disable auto-downloads globally by setting system image to
manually download.  You might want to make the initial check with manual
downloads set, then switch back to automatic downloads for subsequent checks.
It would be a D-Bus API change to specify the download behavior in the
CheckForUpdate call.

>The Entry and PageComponent for System Updates plugins are in fact filtering
>the results to make sure that we show the results only once in case
>concurrent calls are made to checkForUpdates, but the problem here seems to
>be a raise condition between processes when the files that the System Image
>service needs to check if there is in fact an update available are being
>downloaded lots of times and overwritting between each other.  When this was
>tested using the system-image-dbus mock server to reproduce all the different
>scenarios here: https://wiki.ubuntu.com/ImageBasedUpgrades/Client the only
>problem about concurrent calls was that a component could receive multiple
>results of a system update, and that was the fix that was made to filter
>that, but this specific problem didn't show up because probably the mock
>server is not downloading stuff to determine if there is an update.

Be careful about trusting the mock too much.  Mocks are by definition just a
simulation of the real thing, and they do not always accurately mimic the
behavior of the real service.  (I tend to think they cannot possibly do so
without actually *being* the real service.)

For example, currently the service attempts to avoid multiple CheckForUpdate
background actions.  If a check is already in progress, subsequent checks will
never receive an UpdateAvailable signal.  It may be difficult to impossible to
actually do that, because if a check is in progress but not yet completed, we
won't have the information to send yet, and if the check is completed, a
subsequent CheckForUpdate has different semantics so a full second check will
be performed first (but again, because it's highly likely a second check will
find the same update available, it'll use the cached data files and will send
a low cost UpdateAvailable signal).

Think of it like this: you'll get one UpdateAvailable signal for every new or
in-progress update check when CheckForUpdate is called.  If you call CFU while
a check is in progress, you won't get a second UA signal, but if you're
listening, you'll get the first UA response to the in-progress check.

Upon further inspection, I think I can do a better job of blocking multiple
checks from taking place, both within a single process, and guarding against
D-Bus activation starting up system-image more than once.  That, combined with
Manuel's atomic rename change in udm should solve our concurrency problems.

>Could it be possible for the System Image service to know if a current check
>for update is already in progress and avoid triggering that again??  In that
>way, any concurrent service trying to call that won't fail, and each of them
>will receive the notification because the response travels in the same dbus
>signals for each one.

Yes, it's *supposed* to work that way now!  I think there's a small window for
in-process race in system-image and this bug will address that.  As added
protection we'll put a guard in against multiple D-Bus activations.  Just to
reiterate the above, you will get exactly one UpdateAvailable signal per
in-process or new CheckForUpdate operation.  You will not necessarily get one
UpdateAvailable signal per CheckForUpdate call -- i.e. if you call CFU again
while a check is already in progress, you'll see only one UA signal.

Hope that makes sense!