-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Fix deadlock in updateRepository #1813
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
LGTM |
repoInfoSize, err := git.GetRepoSize(repo.RepoPath()) | ||
if err != nil { | ||
return fmt.Errorf("UpdateSize: %v", err) | ||
} | ||
|
||
repo.Size = repoInfoSize.Size + repoInfoSize.SizePack | ||
_, err = x.ID(repo.ID).Cols("size").Update(repo) | ||
_, err = e.Id(repo.ID).Cols("size").Update(repo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ID
is better than Id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ID
is not in the Engine
interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Let's do that later!
A small change required, otherwise LGTM |
Fix a potential deadlock in
updateRepository()
. The call toUpdateSize()
previously did not use the provided session (i.e. thee
parameter), and the resulting query could become blocked by the ongoing session/transaction, since both the query and the ongoing transaction need to access therepository
table.I experienced this deadlock locally, using PostgreSQL.