306961 - App update should handle file sharing violations better [was: Restart should also quit talkback]

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Asaf Romano (gone)]

If talkback is open (due to previous Crash), the mac updater tries to override
the talkback app files while it's running. It looks like we want the internal
restart to also quit talkback.

Comment 1

[Darin Fisher]

How bad is it if we don't fix this?  What happens when we overwrite the talkback
files while it is running?  It sounds like this should block the 1.5 release,
but I just want to make sure I understand what we're dealing with here.  Thx!

Comment 2

[Asaf Romano (gone)]

I'm not sure what's the impact, especially on other OSes.

Comment 3

[Benjamin Smedberg]

On anything but windows, we should be able to write the talkback files and not
obviously affect execution.

On windows, the writing will simply fail (and presumably the app update will fail).

Comment 4

[James Ross]

The only files that will fail on Windows are the EXE and any DLLs it has loaded
(which vary rarely change, by the looks of things). Its data files are fine - I
tried an update with it open, and nothing bad happened, it just switched to the
new build ID when I closed and reopened it.

Comment 5

[Asa Dotzler [:asa]]

So it sounds like this would only be a serious problem (to talkback) if we got
new talkback binaries that we wanted to deliver with app update. Is that
correct? If so, I think that's a pretty rare event and if it did become
necessary, we could try to roll out a fix for this problem before hand (but not
necessarily for 1.5)

Comment 6

[Darin Fisher]

Asa: this could prevent app update from working on windows.  even though we
might not change the talkback binary, if the user is forced to download a
complete update for some reason, then they could be screwed by this bug.

Comment 7

[Asa Dotzler [:asa]]

->darin

Comment 8

[Darin Fisher]

My plan is to make it so that we fail gracefully when we detect that a file is
in use.  Instead of triggering the download of the complete archive, which we do
when the partial update could not be applied due to incompatibilities, we should
just try to apply the partial patch again the next time the user starts their
browser.  We'd want to inform the user that this is going to happen.

Comment 9

[Darin Fisher]

I think that solution would also help with bug 307384.

Comment 10

[Darin Fisher]

Changing the summary and component to reflect the current approach.

Comment 11

[Darin Fisher]

My approach to this problem is to make updater.exe write an error code to
update.status in the failure case.  So, instead of just "failed" it will write
"failed: 2" to indicate an IO error.  Then, Firefox's update service will
inspect this error code and take action accordingly.

Comment 12

[Darin Fisher]

Attachment: v1 patch

OK, this patch does the trick.	I'm not sure if the dialog text is the best,
but it seems reasonable to me.	I'm open to better wording.

Comment 13

[Asa Dotzler [:asa]]

Mike, can you look this over?

Comment 14

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 198086 [details] [diff] [review]
v1 patch

r=[e-mail]

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Wording seems fine for now and for the fact that this is a low-percentage use
case. If you're looking for a little polish, though, we might want to give some
simple hints as to how to fix the problem. Also took the ":" out of the dialog
title and went to one-space-after a period formatting:

+updaterIOErrorTitle=Software Update Failed
+updaterIOErrorText=One or more files could not be updated. Please make sure all
other applications are closed and that you have permission to modify files, and
then restart %S to try again.

BTW, I'm assuming that if this error is encountered twice, we go to download the
full update package, right? Since I can definitely see cases where people will
just never think of shutting down Talkback ...

Comment 16

[Darin Fisher]

Mike: thanks for the wording suggestion.  I like it much better.

> BTW, I'm assuming that if this error is encountered twice, we go to download the
> full update package, right? Since I can definitely see cases where people will
> just never think of shutting down Talkback ...

No, we just keep re-trying.  Downloading the full update would not help if
talkback.exe were running.

Comment 17

[Darin Fisher]

fixed-on-trunk, fixed1.8