Updates apply successfully but will not swap out the folder correctly when a file handle is in use, ending in error: 7

RESOLVED DUPLICATE of bug 760577

Status

()

Toolkit
Application Update
RESOLVED DUPLICATE of bug 760577
6 years ago
6 years ago

People

(Reporter: bbondy, Assigned: bbondy)

Tracking

12 Branch
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
1) Install a build and make sure use service is on
2) Go to the about dialog
3) It downloads an update and applies it fine, the button is changed to Update & Restart
4) If you check maintenanceservice.log it says everything is successful. update.status is changed to applied-service 
5) Click the Update & Restart button
6) Firefox gives an error Message Box prompt with title "Software Update Failed" and message "The update could not be installed.  Please make sure there are no other copies of Firefox running on your computer, and then restart Firefox to try again."
7) At this point maintenanceservice.log contains:
"...
Process was started... waiting on result.
Process finished with return code 1.
updater.exe returned status: failed: 7

Error running update process. Updating update.status Last error: 0
Service command software-update complete.
service command MozillaMaintenance complete with result: Failure."
And update.status contains: failed: 7

update.log contains:
"Performing a replace request
SOURCE DIRECTORY C:\Users\bbondy\AppData\Local\Mozilla\Firefox\Nightly\updates\0
DESTINATION DIRECTORY C:\Program Files (x86)\Nightly\updated
Begin moving sourceDir (C:\Program Files (x86)\Nightly) to tmpDir (C:\Program Files (x86)\Nightly.bak)
rename_file: proceeding to rename the directory
rename_file: failed to rename file - src: C:\Program Files (x86)\Nightly, dst:C:\Program Files (x86)\Nightly.bak, err: 13
Moving sourceDir to tmpDir failed, err: 7
failed: 7
calling QuitProgressUI
Performing a replace request"

8) Click on OK to the messagebox mentioned in #6.  
9) Firefox displays itself after hitting OK. 
update.status contains pending
10) Going back to about has a button that says "Restart to Update"
11) Clicking on it restarts Firefox but no update is there. Going to about again starts downloading again.  Loop back to #1)
(Assignee)

Comment 1

6 years ago
To reproduce this I installed:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-05-23-02-52-50-elm/firefox-15.0a1.en-US.win32.installer.exe

Which is based on: (the first build to include the bgupdates task on elm)
http://hg.mozilla.org/projects/elm/rev/847bb0acebf6

Note: I wasn't getting this before, but I can reproduce it consistently now.  So maybe something is  messed up on my computer, but I don't know what it is.
(Assignee)

Comment 2

6 years ago
> dst:C:\Program Files (x86)\Nightly.bak, err: 13
errno....
EACCES
Permission denied.
An attempt was made to access an object in a way forbidden by its object access permissions.

> Moving sourceDir to tmpDir failed, err: 7
toolkit\mozapps\update\common\errors.h
#define WRITE_ERROR 7
(Assignee)

Comment 3

6 years ago
I think this is because I had a command prompt open at C:\Program Files (x86)\Nightly

Will confirm if this is invalid, testing now.
Yeah, command prompt holds a handle to the directory.
(Assignee)

Comment 5

6 years ago
Yup confirmed it's working on elm now that I closed the command prompt. 

I'll leave it up to you if you want to improve the user experience in another bug but this is not high priority for sure so I'll mark this as invalid.

If we see this error again from someone else we'll know why at least.
(Assignee)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → INVALID
I wouldn't be surprised if some people use a batch file that does this. We should probably leave this open and just replace the contents of the top level directory. We might want to make directory removal non-fatal for this case as well. Re-opening...

Thoughts?
Status: RESOLVED → REOPENED
Resolution: INVALID → ---

Comment 7

6 years ago
(In reply to Robert Strong [:rstrong] (do not email) from comment #6)
> I wouldn't be surprised if some people use a batch file that does this. We
> should probably leave this open and just replace the contents of the top
> level directory. We might want to make directory removal non-fatal for this
> case as well. Re-opening...
> 
> Thoughts?

I don't think replacing the contents of the top level directory is a good idea, since it will be much slower than just replacing the top directory.  There are existing cases where keeping a file open would fail the update, why can't we do the same for the top-level directory?
FWIW, I've hit this error twice today, on my work and home computer. Both of them run Ubuntu 12.04 and have Firefox Nightly installed in /opt/firefox.

/opt/firefox/updates/last-update.log:

......
FINISH ADD Throbber-small.gif
succeeded
calling QuitProgressUI
Performing a replace request
SOURCE DIRECTORY /opt/firefox/updates/0
DESTINATION DIRECTORY /opt/firefox/updated
Begin moving sourceDir (/opt/firefox) to tmpDir (/opt/firefox.bak)
rename_file: proceeding to rename the directory
rename_file: failed to rename file - src: /opt/firefox, dst:/opt/firefox.bak, err: 13
Moving sourceDir to tmpDir failed, err: 7
failed: 7
calling QuitProgressUI

Comment 9

6 years ago
(In reply to Alexander L. Slovesnik from comment #8)
> FWIW, I've hit this error twice today, on my work and home computer. Both of
> them run Ubuntu 12.04 and have Firefox Nightly installed in /opt/firefox.
> 
> /opt/firefox/updates/last-update.log:
> 
> ......
> FINISH ADD Throbber-small.gif
> succeeded
> calling QuitProgressUI
> Performing a replace request
> SOURCE DIRECTORY /opt/firefox/updates/0
> DESTINATION DIRECTORY /opt/firefox/updated
> Begin moving sourceDir (/opt/firefox) to tmpDir (/opt/firefox.bak)
> rename_file: proceeding to rename the directory
> rename_file: failed to rename file - src: /opt/firefox,
> dst:/opt/firefox.bak, err: 13
> Moving sourceDir to tmpDir failed, err: 7
> failed: 7
> calling QuitProgressUI

That's a different bug.  Can you please file another bug and CC me on that?  Note that I suspect that you don't have write permissions on /opt, which is a known failing case for our updater.
(In reply to Ehsan Akhgari [:ehsan] from comment #9)
> (In reply to Alexander L. Slovesnik from comment #8)
> > FWIW, I've hit this error twice today, on my work and home computer. Both of
> > them run Ubuntu 12.04 and have Firefox Nightly installed in /opt/firefox.
> > 
> > /opt/firefox/updates/last-update.log:
> > 
> > ......
> > FINISH ADD Throbber-small.gif
> > succeeded
> > calling QuitProgressUI
> > Performing a replace request
> > SOURCE DIRECTORY /opt/firefox/updates/0
> > DESTINATION DIRECTORY /opt/firefox/updated
> > Begin moving sourceDir (/opt/firefox) to tmpDir (/opt/firefox.bak)
> > rename_file: proceeding to rename the directory
> > rename_file: failed to rename file - src: /opt/firefox,
> > dst:/opt/firefox.bak, err: 13
> > Moving sourceDir to tmpDir failed, err: 7
> > failed: 7
> > calling QuitProgressUI
> 
> That's a different bug.  Can you please file another bug and CC me on that? 
> Note that I suspect that you don't have write permissions on /opt, which is
> a known failing case for our updater.
Filed Bug 757963. I do have write permissions on /opt and I've updated my build previously quite successfully for a few years.

Comment 11

6 years ago
Brian, I think this should be fixed now.  Can you still reproduce it on this build?

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-05-23-09-34-09-oak/

Updated

6 years ago
Blocks: 307181
No longer depends on: 307181
(Assignee)

Comment 12

6 years ago
Still fails, here's what I did:

1) Open up a command prompt to c:\program files (x86)\Nightly
2) Use the build from Comment 11 (http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-05-23-09-34-09-oak/)
3) Go to the about dialog
4) Downloads & Applies fine, click on the button to update and restart
5) You get a MessageBox "Software Update Failed" "The update could not be installed. Please make sure there are no other copies of Firefox running on your computer and then restart Firefox to try again.".  Before pressing OK on the messagebox update.status is set to failed: 7
6) After pressing OK it restarts browser, update.status is set to pending, I noticed no MAR file exists though.
7) Close browser, open browser, repeat from Step 2)

Comment 13

6 years ago
Hmm, OK, do you guys think this should block re-enabling background updates on mozilla-central (bug 758101) then?  I don't.

But I'll investigate this.
Assignee: nobody → ehsan
Status: REOPENED → ASSIGNED
(In reply to Brian R. Bondy [:bbondy] from comment #12)
> Still fails, here's what I did:
> 
> 1) Open up a command prompt to c:\program files (x86)\Nightly
> 2) Use the build from Comment 11
> (http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-05-23-09-34-09-
> oak/)
> 3) Go to the about dialog
> 4) Downloads & Applies fine, click on the button to update and restart
> 5) You get a MessageBox "Software Update Failed" "The update could not be
> installed. Please make sure there are no other copies of Firefox running on
> your computer and then restart Firefox to try again.".  Before pressing OK
> on the messagebox update.status is set to failed: 7
> 6) After pressing OK it restarts browser, update.status is set to pending, I
> noticed no MAR file exists though.
> 7) Close browser, open browser, repeat from Step 2)
I *think* this is due to the command prompt having an open handle to "c:\program files (x86)\Nightly". If this is confirmed to be the case then I am ok with this not blocking re-enabling and having a followup bug for this issue.

Comment 15

6 years ago
(In reply to Robert Strong [:rstrong] (do not email) from comment #14)
> (In reply to Brian R. Bondy [:bbondy] from comment #12)
> > Still fails, here's what I did:
> > 
> > 1) Open up a command prompt to c:\program files (x86)\Nightly
> > 2) Use the build from Comment 11
> > (http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-05-23-09-34-09-
> > oak/)
> > 3) Go to the about dialog
> > 4) Downloads & Applies fine, click on the button to update and restart
> > 5) You get a MessageBox "Software Update Failed" "The update could not be
> > installed. Please make sure there are no other copies of Firefox running on
> > your computer and then restart Firefox to try again.".  Before pressing OK
> > on the messagebox update.status is set to failed: 7
> > 6) After pressing OK it restarts browser, update.status is set to pending, I
> > noticed no MAR file exists though.
> > 7) Close browser, open browser, repeat from Step 2)
> I *think* this is due to the command prompt having an open handle to
> "c:\program files (x86)\Nightly". If this is confirmed to be the case then I
> am ok with this not blocking re-enabling and having a followup bug for this
> issue.

Yeah, that is the cause behind this bug.
(Assignee)

Updated

6 years ago
Summary: Updates apply successfully but will not swap out the folder correctly ending in error: 7 → Updates apply successfully but will not swap out the folder correctly when a file handle is in use, ending in error: 7
(Assignee)

Updated

6 years ago
Duplicate of this bug: 760206
(Assignee)

Comment 17

6 years ago
Created attachment 629027 [details] [diff] [review]
Patch v1.

So this patch checks the installation directory to see if there are any other open file handles to that directory.

The file handle is opened with FILE_FLAG_BACKUP_SEMANTICS because this is needed to get a handle for a directory.  It is also opened with GENERIC_READ (in contrast to needing write access) because it will work with both elevated locations and unelevated locations. 

The check works by specifying 0 sharing rights, if something is already open which is restricting access that will return an invalid handle with a sharing violation last error. 

I'll test this on Oak as well.
Assignee: ehsan → netzen
Attachment #629027 - Flags: review?(ehsan)
(Assignee)

Updated

6 years ago
Keywords: dev-doc-needed

Comment 18

6 years ago
Comment on attachment 629027 [details] [diff] [review]
Patch v1.

Review of attachment 629027 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!
Attachment #629027 - Flags: review?(ehsan) → review+
(Assignee)

Comment 19

6 years ago
This check works perfect when you are installed into a location that has a low integrity level (like the user's app dir), but it doesn't work when installed in a higher integrity level like Program files.  Trying to find a check that will work in both cases now.
(Assignee)

Comment 20

6 years ago
I presume the reason why it doesn't work for high integrity directories is because a low integrity process cannot modify it's delete sharing permissions.  So when we pass in no sharing rights, that would normally conflict with the open handle (which already disallowed FILE_SHARE_DELETE).  But since we are a low integrity process it ignores that flag.

Comment 21

6 years ago
Ah, crap.  Do you know of another way to make this check (I don't think I do)?
(Assignee)

Comment 22

6 years ago
Looking into it now, currently this patch will only help users that install into their app dir, which is probably not most users.  I tested this code out on my desktop with a subfolder there which is low integrity level.
(Assignee)

Comment 23

6 years ago
Also the patch will currently help XP users and people with UAC off.
(Assignee)

Comment 24

6 years ago
We could use NTQueryObject and check to make sure the handle count for the file is 0, but it is an internal API and may change or be removed.  The removed part I'm not afraid of, the changed part may cause problems in the future.  I think it is unlikely that it would change though.

Are you opposed to allow using NtQueryObject?
http://msdn.microsoft.com/en-us/library/bb432383%28v=vs.85%29.aspx
(Assignee)

Comment 25

6 years ago
I think all of these checks will be too restrictive anyway when they do work because our process itself (firefox.exe) can sometimes have a handle to the dir when it is started from a shortcut especially.  So I think we shouldn't land this at all, I'll back it out of oak and I'll cancel the r+.
(Assignee)

Updated

6 years ago
Attachment #629027 - Flags: review+
(Assignee)

Updated

6 years ago
Keywords: dev-doc-needed

Comment 26

6 years ago
Yeah, I agree with comment 25 now.  The correct way to fix this is to fix bug 760577.  So I'm marking this as a dupe of that bug.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 760577
Duplicate of this bug: 761248
You need to log in before you can comment on or make changes to this bug.