Failed to update size for repository by a race condition #10078

Closed
opened 2025-11-02 08:57:40 -06:00 by GiteaMirror · 1 comment
Owner

Originally created by @fsologureng on GitHub (Jan 9, 2023).

Description

2023/01/06 22:05:35 ...s/repository/push.go:100:pushUpdates() [E] [63b89b2f-14] Failed to update size for repository: updateSize: lstat /mnt/ceph-cluster/git/gitea-repositories/<user>/blog.git/refs/heads/<somefile>.lock: no such file or directory

There is a race condition regarding updating the repo size while processing pushes.

It is observed:

  1. there is no protection in func GetDirectorySize regarding the existence of the path. If the path does not exist, a response of size 0 would be a good response so as not to cause an error. For example, this error 500 is thrown due to bad code in the caller.
  2. There is no exclusion for lock files. If concurrency is not even implemented, then a straightforward solution is to exclude lock files (their existence implying a not handled race condition), allowing the wrong size to be fetched, in favour of not failing an important thread just because of the size calculation. filepath.WalkFunc could handle this case by checking the os.Lstat error and the lock suffix of the filename. For example this error handling is done by another very similar code with the suggested protection but even more general than excluding non-existing lock files.
  3. The Go developers suggest using filepath.WalkDir instead of filepath.Walk because *Walk is less efficient than WalkDir, introduced in Go 1.16, which avoids calling os.Lstat on every file or directory visited. The call uses a different walk function, but is very easy to adapt.

Originally reported in https://codeberg.org/forgejo/forgejo/issues/216

Gitea Version

1.18.0

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

systemd

Database

None

Originally created by @fsologureng on GitHub (Jan 9, 2023). ### Description ``` 2023/01/06 22:05:35 ...s/repository/push.go:100:pushUpdates() [E] [63b89b2f-14] Failed to update size for repository: updateSize: lstat /mnt/ceph-cluster/git/gitea-repositories/<user>/blog.git/refs/heads/<somefile>.lock: no such file or directory ``` There is a race condition regarding updating the repo size while processing pushes. It is observed: 1. there is no protection in func [`GetDirectorySize`](https://github.com/go-gitea/gitea/blob/main/modules/util/path.go#L28) regarding the existence of the path. If the path does not exist, a response of size 0 would be a good response so as not to cause an error. For example, [this error 500](https://github.com/go-gitea/gitea/issues/14517) is thrown due to bad code in the caller. 1. There is no exclusion for lock files. If concurrency is not even implemented, then a straightforward solution is to exclude lock files (their existence implying a not handled race condition), allowing the wrong size to be fetched, in favour of not failing an important thread just because of the size calculation. `filepath.WalkFunc` could handle this case by checking the `os.Lstat` error and the `lock` suffix of the filename. For example [this error handling](https://github.com/go-gitea/gitea/blame/main/routers/web/user/setting/profile.go#L285) is done by another very similar code with the suggested protection but even more general than excluding non-existing lock files. 1. The Go developers suggest using `filepath.WalkDir` instead of `filepath.Walk` because [*Walk is less efficient than WalkDir, introduced in Go 1.16, which avoids calling `os.Lstat` on every file or directory visited](https://pkg.go.dev/path/filepath#Walk). The call uses a different walk function, but is very easy to adapt. Originally reported in https://codeberg.org/forgejo/forgejo/issues/216 ### Gitea Version 1.18.0 ### Can you reproduce the bug on the Gitea demo site? No ### Log Gist _No response_ ### Screenshots _No response_ ### Git Version _No response_ ### Operating System _No response_ ### How are you running Gitea? systemd ### Database None
GiteaMirror added the type/bug label 2025-11-02 08:57:40 -06:00
Author
Owner

@lunny commented on GitHub (Jan 10, 2023):

Take a look at the lock file comment. 2a97289ad8/lockfile.h (L9-L16)
Because one go routine are calculating the file size and another are writing some files. I think we can simple ignore a *.lock file not exist. And filepath.WalkDir is a better way to do walk. Some code are from old Golang, so they are still using filepath.Walk.

@lunny commented on GitHub (Jan 10, 2023): Take a look at the lock file comment. https://github.com/git/git/blob/2a97289ad8b103625d3a1a12f66c27f50df822ce/lockfile.h#L9-L16 Because one go routine are calculating the file size and another are writing some files. I think we can simple ignore a *.lock file not exist. And `filepath.WalkDir` is a better way to do walk. Some code are from old Golang, so they are still using `filepath.Walk`.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#10078