From 05d4a69243281c48c0f56b75e838575ee7eb20b1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 15 Jul 2021 13:38:10 +0100 Subject: [PATCH 1/2] 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 --- cmd/embedded.go | 2 +- models/repo.go | 4 ++-- models/repo_transfer.go | 8 ++++---- models/ssh_key.go | 4 ++-- models/user.go | 4 ++-- modules/log/file.go | 2 +- modules/storage/local.go | 2 +- modules/util/remove.go | 29 ++++++++++++++++++++++++++++- 8 files changed, 41 insertions(+), 14 deletions(-) diff --git a/cmd/embedded.go b/cmd/embedded.go index 363b85c066eaa..43ead915f1832 100644 --- a/cmd/embedded.go +++ b/cmd/embedded.go @@ -271,7 +271,7 @@ func extractAsset(d string, a asset, overwrite, rename bool) error { } else if !fi.Mode().IsRegular() { return fmt.Errorf("%s already exists, but it's not a regular file", dest) } 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) } // Attempt to respect file permissions mask (even if user:group will be set anew) diff --git a/models/repo.go b/models/repo.go index 143dff9ac3490..d6abc1b5e3883 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1227,7 +1227,7 @@ func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err } 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) } @@ -1238,7 +1238,7 @@ func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err return err } 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) } } diff --git a/models/repo_transfer.go b/models/repo_transfer.go index c5d1a3a3c2172..d7ef0a8ca6b3b 100644 --- a/models/repo_transfer.go +++ b/models/repo_transfer.go @@ -210,13 +210,13 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e } 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) } } 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) } } @@ -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) } - 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) } 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) return err } 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) } wikiRenamed = true diff --git a/models/ssh_key.go b/models/ssh_key.go index e35fc12e080a2..12c7bc91162ae 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -842,7 +842,7 @@ func rewriteAllPublicKeys(e Engine) error { } t.Close() - return os.Rename(tmpPath, fPath) + return util.Rename(tmpPath, fPath) } // RegeneratePublicKeys regenerates the authorized_keys file @@ -1324,7 +1324,7 @@ func rewriteAllPrincipalKeys(e Engine) error { } t.Close() - return os.Rename(tmpPath, fPath) + return util.Rename(tmpPath, fPath) } // ListPrincipalKeys returns a list of principals belongs to given user. diff --git a/models/user.go b/models/user.go index 3b8ce79251f0c..f606da53d65f0 100644 --- a/models/user.go +++ b/models/user.go @@ -1027,7 +1027,7 @@ func ChangeUserName(u *User, newUserName string) (err error) { } // 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) } @@ -1036,7 +1036,7 @@ func ChangeUserName(u *User, newUserName string) (err error) { } 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) 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) } diff --git a/modules/log/file.go b/modules/log/file.go index d5b38d4e0172d..79cbe740fdb8f 100644 --- a/modules/log/file.go +++ b/modules/log/file.go @@ -177,7 +177,7 @@ func (log *FileLogger) DoRotate() error { // close fd before rename // 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) } diff --git a/modules/storage/local.go b/modules/storage/local.go index 46e5d60e6b5d3..1329f722c285e 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -96,7 +96,7 @@ func (l *LocalStorage) Save(path string, r io.Reader, size int64) (int64, error) return 0, err } - if err := os.Rename(tmp.Name(), p); err != nil { + if err := util.Rename(tmp.Name(), p); err != nil { return 0, err } diff --git a/modules/util/remove.go b/modules/util/remove.go index f2bbbc30b928c..23104365256fc 100644 --- a/modules/util/remove.go +++ b/modules/util/remove.go @@ -33,7 +33,7 @@ func Remove(name string) error { 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 { var err error for i := 0; i < 5; i++ { @@ -55,3 +55,30 @@ func RemoveAll(name string) error { } 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 +} From 62d994f139a51a193f48d5c279a55bd8887a510b Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Thu, 15 Jul 2021 11:07:07 -0400 Subject: [PATCH 2/2] resolve CI fail --- cmd/embedded.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/embedded.go b/cmd/embedded.go index 43ead915f1832..528f32402e1a2 100644 --- a/cmd/embedded.go +++ b/cmd/embedded.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/modules/public" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/modules/util" "github.com/gobwas/glob" "github.com/urfave/cli"