Skip to content

Commit 33a8eec

Browse files
Retry rename on lock induced failures (go-gitea#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 go-gitea#16427 Signed-off-by: Andrew Thornton <[email protected]> * resolve CI fail Co-authored-by: techknowlogick <[email protected]>
1 parent aed086f commit 33a8eec

File tree

8 files changed

+42
-14
lines changed

8 files changed

+42
-14
lines changed

cmd/embedded.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"code.gitea.io/gitea/modules/public"
2020
"code.gitea.io/gitea/modules/setting"
2121
"code.gitea.io/gitea/modules/templates"
22+
"code.gitea.io/gitea/modules/util"
2223

2324
"github.com/gobwas/glob"
2425
"github.com/urfave/cli"
@@ -271,7 +272,7 @@ func extractAsset(d string, a asset, overwrite, rename bool) error {
271272
} else if !fi.Mode().IsRegular() {
272273
return fmt.Errorf("%s already exists, but it's not a regular file", dest)
273274
} else if rename {
274-
if err := os.Rename(dest, dest+".bak"); err != nil {
275+
if err := util.Rename(dest, dest+".bak"); err != nil {
275276
return fmt.Errorf("Error creating backup for %s: %v", dest, err)
276277
}
277278
// Attempt to respect file permissions mask (even if user:group will be set anew)

models/repo.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1227,7 +1227,7 @@ func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err
12271227
}
12281228

12291229
newRepoPath := RepoPath(repo.Owner.Name, newRepoName)
1230-
if err = os.Rename(repo.RepoPath(), newRepoPath); err != nil {
1230+
if err = util.Rename(repo.RepoPath(), newRepoPath); err != nil {
12311231
return fmt.Errorf("rename repository directory: %v", err)
12321232
}
12331233

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

models/repo_transfer.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -210,13 +210,13 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
210210
}
211211

212212
if repoRenamed {
213-
if err := os.Rename(RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name)); err != nil {
213+
if err := util.Rename(RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name)); err != nil {
214214
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)
215215
}
216216
}
217217

218218
if wikiRenamed {
219-
if err := os.Rename(WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name)); err != nil {
219+
if err := util.Rename(WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name)); err != nil {
220220
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)
221221
}
222222
}
@@ -358,7 +358,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
358358
return fmt.Errorf("Failed to create dir %s: %v", dir, err)
359359
}
360360

361-
if err := os.Rename(RepoPath(oldOwner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil {
361+
if err := util.Rename(RepoPath(oldOwner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil {
362362
return fmt.Errorf("rename repository directory: %v", err)
363363
}
364364
repoRenamed = true
@@ -370,7 +370,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
370370
log.Error("Unable to check if %s exists. Error: %v", wikiPath, err)
371371
return err
372372
} else if isExist {
373-
if err := os.Rename(wikiPath, WikiPath(newOwner.Name, repo.Name)); err != nil {
373+
if err := util.Rename(wikiPath, WikiPath(newOwner.Name, repo.Name)); err != nil {
374374
return fmt.Errorf("rename repository wiki: %v", err)
375375
}
376376
wikiRenamed = true

models/ssh_key.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,7 @@ func rewriteAllPublicKeys(e Engine) error {
842842
}
843843

844844
t.Close()
845-
return os.Rename(tmpPath, fPath)
845+
return util.Rename(tmpPath, fPath)
846846
}
847847

848848
// RegeneratePublicKeys regenerates the authorized_keys file
@@ -1324,7 +1324,7 @@ func rewriteAllPrincipalKeys(e Engine) error {
13241324
}
13251325

13261326
t.Close()
1327-
return os.Rename(tmpPath, fPath)
1327+
return util.Rename(tmpPath, fPath)
13281328
}
13291329

13301330
// ListPrincipalKeys returns a list of principals belongs to given user.

models/user.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1027,7 +1027,7 @@ func ChangeUserName(u *User, newUserName string) (err error) {
10271027
}
10281028

10291029
// Do not fail if directory does not exist
1030-
if err = os.Rename(UserPath(oldUserName), UserPath(newUserName)); err != nil && !os.IsNotExist(err) {
1030+
if err = util.Rename(UserPath(oldUserName), UserPath(newUserName)); err != nil && !os.IsNotExist(err) {
10311031
return fmt.Errorf("Rename user directory: %v", err)
10321032
}
10331033

@@ -1036,7 +1036,7 @@ func ChangeUserName(u *User, newUserName string) (err error) {
10361036
}
10371037

10381038
if err = sess.Commit(); err != nil {
1039-
if err2 := os.Rename(UserPath(newUserName), UserPath(oldUserName)); err2 != nil && !os.IsNotExist(err2) {
1039+
if err2 := util.Rename(UserPath(newUserName), UserPath(oldUserName)); err2 != nil && !os.IsNotExist(err2) {
10401040
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)
10411041
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)
10421042
}

modules/log/file.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func (log *FileLogger) DoRotate() error {
177177

178178
// close fd before rename
179179
// Rename the file to its newfound home
180-
if err = os.Rename(log.Filename, fname); err != nil {
180+
if err = util.Rename(log.Filename, fname); err != nil {
181181
return fmt.Errorf("Rotate: %v", err)
182182
}
183183

modules/storage/local.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func (l *LocalStorage) Save(path string, r io.Reader, size int64) (int64, error)
9696
return 0, err
9797
}
9898

99-
if err := os.Rename(tmp.Name(), p); err != nil {
99+
if err := util.Rename(tmp.Name(), p); err != nil {
100100
return 0, err
101101
}
102102

modules/util/remove.go

+28-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func Remove(name string) error {
3333
return err
3434
}
3535

36-
// RemoveAll removes the named file or (empty) directory with at most 5 attempts.Remove
36+
// RemoveAll removes the named file or (empty) directory with at most 5 attempts.
3737
func RemoveAll(name string) error {
3838
var err error
3939
for i := 0; i < 5; i++ {
@@ -55,3 +55,30 @@ func RemoveAll(name string) error {
5555
}
5656
return err
5757
}
58+
59+
// Rename renames (moves) oldpath to newpath with at most 5 attempts.
60+
func Rename(oldpath, newpath string) error {
61+
var err error
62+
for i := 0; i < 5; i++ {
63+
err = os.Rename(oldpath, newpath)
64+
if err == nil {
65+
break
66+
}
67+
unwrapped := err.(*os.PathError).Err
68+
if unwrapped == syscall.EBUSY || unwrapped == syscall.ENOTEMPTY || unwrapped == syscall.EPERM || unwrapped == syscall.EMFILE || unwrapped == syscall.ENFILE {
69+
// try again
70+
<-time.After(100 * time.Millisecond)
71+
continue
72+
}
73+
74+
if i == 0 && os.IsNotExist(err) {
75+
return err
76+
}
77+
78+
if unwrapped == syscall.ENOENT {
79+
// it's already gone
80+
return nil
81+
}
82+
}
83+
return err
84+
}

0 commit comments

Comments
 (0)