Open Bug 1703852 Opened 3 years ago Updated 3 years ago

The BUA seems to fall behind a version if the task scheduler ran but failed the update execution

Categories

(Toolkit :: Application Update, defect)

Firefox 89
All
Windows
defect

Tracking

()

Tracking Status
firefox89 --- affected

People

(Reporter: asoncutean, Unassigned)

References

(Blocks 1 open bug)

Details

Affected versions

  • 89.0a1

Affected platforms

  • Windows 10

Steps to reproduce

  1. Install an older version of Firefox (+89)
  2. Go to the default profile, inside about:config set:
  • app.update.log=true
  • app.update.background.loglevel=debug
  • app.update.background.experimental=true
  1. Navigate to about:preferences and check the When Firefox/Nightly is not running option inside the Update section
  2. Close the browser
  3. Make sure the Background update agent task is periodically run inside Task scheduler
  4. Check the logs periodically

Expected result

  • Firefox is updated to the latest available version

Actual result

  • The Agent updated Firefox periodically for 3 versions
  • While in sleep mode the task performed but fail to update to the latest version (from 20210406152948 to 20210407094544)
  • Task is scheduled for the next 7h, but at the time of the execution task didn't run (the OS was opened, Firefox closed) - see screenshot
  • Task is scheduled for the next 7h, at the time of execution it is ran properly, updates the Firefox, but to the second to last version available (to 20210407094544 instead of 20210407212527)
  • Ran the task scheduler manually after version 20210408095111 was available but the updated stopped at the previous one 20210407212527)

Regression range

  • Not a regression

Additional notes

  • See the attached logs
  • This needs further investigation, but it doesn’t seem to be applied as a general pattern

Sadly the attached logs are partial/corrupted. While I haven't seen this myself, :agashlin and :bytesized have. We're going to address it by making the MOZ_LOG parameters include sync: Bug 1703877, which I've just landed.

Now, I'm fairly sure that this is fine and expected. What I think is happening is that the background task has fetched the update 20210407094544 but failed to stage/apply it. When the task starts again, it completes that update even though there's a newer one available. That's known and expected. And based on the description I think that happened again with 20210407212527 to 20210408095111.

Basically, the task will fetch and stage update N on one iteration, and then apply it and fetch and stage update N+1 on the next iteration. We could be more aggressive here, trying to restart the task immediately to finish applying more quickly, but non-Nightly settings this conservative approach should be just fine.

:bytesized, is my analysis correct? Does the multi-update downloading work from Bug 353804 impact this?

Flags: needinfo?(ksteuber)

(In reply to Nick Alexander :nalexander [he/him] from comment #1)

Basically, the task will fetch and stage update N on one iteration, and then apply it and fetch and stage update N+1 on the next iteration.

:bytesized, is my analysis correct?

That sounds right. The only wrinkle is that each "iteration" is a little fuzzy because they can kind of overlap a bit. On the first run, we start downloading. On the second run, (assuming downloading has finished) we stage the update. On the third run, the update will be installed, the (updated) task gets automatically relaunched, and then immediately kicks off the download of the next update (assuming one is available).

Does the multi-update downloading work from Bug 353804 impact this?

Hmm... it might be possible for us to hit that in sort of a weird way, but I don't think that is happening here.

We might want to prevent that actually, since it may work in an unexpected manner.

I was going to say that it was impossible, because background update only ever updates via AppUpdater, which has no interface into the multi-update mechanism. That mechanism can only ever be entered through timer-initiated update checks "in the background", not as a result of user actions (which is what AppUpdater is normally used for).

However, it might be possible to get into a state where that happens. If Firefox gets sufficiently out-of-date, we kick off an update much earlier than usual (here, I think). We should probably prevent Background Tasks from entering the update system through AUS._checkForBackgroundUpdates so that we don't end up in an unexpected flow.

Flags: needinfo?(ksteuber)

However, it might be possible to get into a state where that happens. If Firefox gets sufficiently out-of-date, we kick off an update much earlier than usual (here, I think). We should probably prevent Background Tasks from entering the update system through AUS._checkForBackgroundUpdates so that we don't end up in an unexpected flow.

No need: nothing in BrowserGlue.jsm is processed in background task mode. That's basically how we avoid starting the browser proper: see https://firefox-source-docs.mozilla.org/toolkit/components/backgroundtasks/index.html#background-tasks-limit-the-xpcom-instance-graph-by-default for a tiny blurb about this.

This mechanism was essentially the cause of https://bugzilla.mozilla.org/show_bug.cgi?id=1700987, too.

(In reply to Nick Alexander :nalexander [he/him] from comment #3)

No need: nothing in BrowserGlue.jsm is processed in background task mode.

Ah, excellent. Would it be worth adding a guard to that function just to be safe? I only ask because of the other way of getting into that function: the update timer. The way it works is a bit weird, and I don't actually understand how it gets loaded. And given the way that we are copying pref values around from the default profile, I can't easily reason out whether there is some way for it to fire in a Background Update Task.

Severity: -- → S3
Has STR: --- → yes

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #4)

(In reply to Nick Alexander :nalexander [he/him] from comment #3)

No need: nothing in BrowserGlue.jsm is processed in background task mode.

Ah, excellent. Would it be worth adding a guard to that function just to be safe? I only ask because of the other way of getting into that function: the update timer. The way it works is a bit weird, and I don't actually understand how it gets loaded. And given the way that we are copying pref values around from the default profile, I can't easily reason out whether there is some way for it to fire in a Background Update Task.

The timers won't be fired in background task mode due to the same mechanism: the category registrations aren't processed. So unless there's another way for them to be loaded, I don't think we'd need to do anything more.

You need to log in before you can comment on or make changes to this bug.