Retry rename on lock induced failures (#16435)

* Retry rename on lock induced failures

Due to external locking on Windows it is possible for an
os.Rename to fail if the files or directories are being
used elsewhere.

This PR simply suggests retrying the rename again similar
to how we handle the os.Remove problems.

Fix #16427

Signed-off-by: Andrew Thornton <art27@cantab.net>

* resolve CI fail

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
tokarchuk/v1.17
zeripath 3 years ago committed by GitHub
parent aed086f8b0
commit 33a8eec33e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 3
      cmd/embedded.go
  2. 4
      models/repo.go
  3. 8
      models/repo_transfer.go
  4. 4
      models/ssh_key.go
  5. 4
      models/user.go
  6. 2
      modules/log/file.go
  7. 2
      modules/storage/local.go
  8. 29
      modules/util/remove.go

@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/modules/public" "code.gitea.io/gitea/modules/public"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/util"
"github.com/gobwas/glob" "github.com/gobwas/glob"
"github.com/urfave/cli" "github.com/urfave/cli"
@ -271,7 +272,7 @@ func extractAsset(d string, a asset, overwrite, rename bool) error {
} else if !fi.Mode().IsRegular() { } else if !fi.Mode().IsRegular() {
return fmt.Errorf("%s already exists, but it's not a regular file", dest) return fmt.Errorf("%s already exists, but it's not a regular file", dest)
} else if rename { } else if rename {
if err := os.Rename(dest, dest+".bak"); err != nil { if err := util.Rename(dest, dest+".bak"); err != nil {
return fmt.Errorf("Error creating backup for %s: %v", dest, err) return fmt.Errorf("Error creating backup for %s: %v", dest, err)
} }
// Attempt to respect file permissions mask (even if user:group will be set anew) // Attempt to respect file permissions mask (even if user:group will be set anew)

@ -1227,7 +1227,7 @@ func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err
} }
newRepoPath := RepoPath(repo.Owner.Name, newRepoName) newRepoPath := RepoPath(repo.Owner.Name, newRepoName)
if err = os.Rename(repo.RepoPath(), newRepoPath); err != nil { if err = util.Rename(repo.RepoPath(), newRepoPath); err != nil {
return fmt.Errorf("rename repository directory: %v", err) return fmt.Errorf("rename repository directory: %v", err)
} }
@ -1238,7 +1238,7 @@ func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err
return err return err
} }
if isExist { if isExist {
if err = os.Rename(wikiPath, WikiPath(repo.Owner.Name, newRepoName)); err != nil { if err = util.Rename(wikiPath, WikiPath(repo.Owner.Name, newRepoName)); err != nil {
return fmt.Errorf("rename repository wiki: %v", err) return fmt.Errorf("rename repository wiki: %v", err)
} }
} }

@ -210,13 +210,13 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
} }
if repoRenamed { if repoRenamed {
if err := os.Rename(RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name)); err != nil { if err := util.Rename(RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name)); err != nil {
log.Critical("Unable to move repository %s/%s directory from %s back to correct place %s: %v", oldOwnerName, repo.Name, RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name), err) log.Critical("Unable to move repository %s/%s directory from %s back to correct place %s: %v", oldOwnerName, repo.Name, RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name), err)
} }
} }
if wikiRenamed { if wikiRenamed {
if err := os.Rename(WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name)); err != nil { if err := util.Rename(WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name)); err != nil {
log.Critical("Unable to move wiki for repository %s/%s directory from %s back to correct place %s: %v", oldOwnerName, repo.Name, WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name), err) log.Critical("Unable to move wiki for repository %s/%s directory from %s back to correct place %s: %v", oldOwnerName, repo.Name, WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name), err)
} }
} }
@ -358,7 +358,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
return fmt.Errorf("Failed to create dir %s: %v", dir, err) return fmt.Errorf("Failed to create dir %s: %v", dir, err)
} }
if err := os.Rename(RepoPath(oldOwner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil { if err := util.Rename(RepoPath(oldOwner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil {
return fmt.Errorf("rename repository directory: %v", err) return fmt.Errorf("rename repository directory: %v", err)
} }
repoRenamed = true repoRenamed = true
@ -370,7 +370,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
log.Error("Unable to check if %s exists. Error: %v", wikiPath, err) log.Error("Unable to check if %s exists. Error: %v", wikiPath, err)
return err return err
} else if isExist { } else if isExist {
if err := os.Rename(wikiPath, WikiPath(newOwner.Name, repo.Name)); err != nil { if err := util.Rename(wikiPath, WikiPath(newOwner.Name, repo.Name)); err != nil {
return fmt.Errorf("rename repository wiki: %v", err) return fmt.Errorf("rename repository wiki: %v", err)
} }
wikiRenamed = true wikiRenamed = true

@ -842,7 +842,7 @@ func rewriteAllPublicKeys(e Engine) error {
} }
t.Close() t.Close()
return os.Rename(tmpPath, fPath) return util.Rename(tmpPath, fPath)
} }
// RegeneratePublicKeys regenerates the authorized_keys file // RegeneratePublicKeys regenerates the authorized_keys file
@ -1324,7 +1324,7 @@ func rewriteAllPrincipalKeys(e Engine) error {
} }
t.Close() t.Close()
return os.Rename(tmpPath, fPath) return util.Rename(tmpPath, fPath)
} }
// ListPrincipalKeys returns a list of principals belongs to given user. // ListPrincipalKeys returns a list of principals belongs to given user.

@ -1027,7 +1027,7 @@ func ChangeUserName(u *User, newUserName string) (err error) {
} }
// Do not fail if directory does not exist // Do not fail if directory does not exist
if err = os.Rename(UserPath(oldUserName), UserPath(newUserName)); err != nil && !os.IsNotExist(err) { if err = util.Rename(UserPath(oldUserName), UserPath(newUserName)); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("Rename user directory: %v", err) return fmt.Errorf("Rename user directory: %v", err)
} }
@ -1036,7 +1036,7 @@ func ChangeUserName(u *User, newUserName string) (err error) {
} }
if err = sess.Commit(); err != nil { if err = sess.Commit(); err != nil {
if err2 := os.Rename(UserPath(newUserName), UserPath(oldUserName)); err2 != nil && !os.IsNotExist(err2) { if err2 := util.Rename(UserPath(newUserName), UserPath(oldUserName)); err2 != nil && !os.IsNotExist(err2) {
log.Critical("Unable to rollback directory change during failed username change from: %s to: %s. DB Error: %v. Filesystem Error: %v", oldUserName, newUserName, err, err2) log.Critical("Unable to rollback directory change during failed username change from: %s to: %s. DB Error: %v. Filesystem Error: %v", oldUserName, newUserName, err, err2)
return fmt.Errorf("failed to rollback directory change during failed username change from: %s to: %s. DB Error: %w. Filesystem Error: %v", oldUserName, newUserName, err, err2) return fmt.Errorf("failed to rollback directory change during failed username change from: %s to: %s. DB Error: %w. Filesystem Error: %v", oldUserName, newUserName, err, err2)
} }

@ -177,7 +177,7 @@ func (log *FileLogger) DoRotate() error {
// close fd before rename // close fd before rename
// Rename the file to its newfound home // Rename the file to its newfound home
if err = os.Rename(log.Filename, fname); err != nil { if err = util.Rename(log.Filename, fname); err != nil {
return fmt.Errorf("Rotate: %v", err) return fmt.Errorf("Rotate: %v", err)
} }

@ -96,7 +96,7 @@ func (l *LocalStorage) Save(path string, r io.Reader, size int64) (int64, error)
return 0, err return 0, err
} }
if err := os.Rename(tmp.Name(), p); err != nil { if err := util.Rename(tmp.Name(), p); err != nil {
return 0, err return 0, err
} }

@ -33,7 +33,7 @@ func Remove(name string) error {
return err return err
} }
// RemoveAll removes the named file or (empty) directory with at most 5 attempts.Remove // RemoveAll removes the named file or (empty) directory with at most 5 attempts.
func RemoveAll(name string) error { func RemoveAll(name string) error {
var err error var err error
for i := 0; i < 5; i++ { for i := 0; i < 5; i++ {
@ -55,3 +55,30 @@ func RemoveAll(name string) error {
} }
return err return err
} }
// Rename renames (moves) oldpath to newpath with at most 5 attempts.
func Rename(oldpath, newpath string) error {
var err error
for i := 0; i < 5; i++ {
err = os.Rename(oldpath, newpath)
if err == nil {
break
}
unwrapped := err.(*os.PathError).Err
if unwrapped == syscall.EBUSY || unwrapped == syscall.ENOTEMPTY || unwrapped == syscall.EPERM || unwrapped == syscall.EMFILE || unwrapped == syscall.ENFILE {
// try again
<-time.After(100 * time.Millisecond)
continue
}
if i == 0 && os.IsNotExist(err) {
return err
}
if unwrapped == syscall.ENOENT {
// it's already gone
return nil
}
}
return err
}

Loading…
Cancel
Save