From 6311e4ce6aba67d15a199d94adbdb81f9acbe9ce Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 4 Jan 2019 21:51:27 +0000 Subject: [PATCH] Fix sqlite deadlock when assigning to a PR (#5640) * Fix sqlite deadlock when assigning to a PR Fix 5639 Signed-off-by: Andrew Thornton * More possible deadlocks found and fixed Signed-off-by: Andrew Thornton --- models/issue.go | 2 +- models/issue_assignees.go | 8 ++++++-- models/issue_mail.go | 2 +- models/issue_user.go | 2 +- models/org.go | 6 +++++- models/repo_watch.go | 6 +++--- 6 files changed, 17 insertions(+), 9 deletions(-) diff --git a/models/issue.go b/models/issue.go index cee9b0ca5..c982931c0 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1402,7 +1402,7 @@ func UpdateIssueMentions(e Engine, issueID int64, mentions []string) error { } memberIDs := make([]int64, 0, user.NumMembers) - orgUsers, err := GetOrgUsersByOrgID(user.ID) + orgUsers, err := getOrgUsersByOrgID(e, user.ID) if err != nil { return fmt.Errorf("GetOrgUsersByOrgID [%d]: %v", user.ID, err) } diff --git a/models/issue_assignees.go b/models/issue_assignees.go index 753396e39..0fff2dfad 100644 --- a/models/issue_assignees.go +++ b/models/issue_assignees.go @@ -44,7 +44,11 @@ func (issue *Issue) loadAssignees(e Engine) (err error) { // GetAssigneesByIssue returns everyone assigned to that issue func GetAssigneesByIssue(issue *Issue) (assignees []*User, err error) { - err = issue.loadAssignees(x) + return getAssigneesByIssue(x, issue) +} + +func getAssigneesByIssue(e Engine, issue *Issue) (assignees []*User, err error) { + err = issue.loadAssignees(e) if err != nil { return assignees, err } @@ -173,7 +177,7 @@ func (issue *Issue) changeAssignee(sess *xorm.Session, doer *User, assigneeID in issue.PullRequest.Issue = issue apiPullRequest := &api.PullRequestPayload{ Index: issue.Index, - PullRequest: issue.PullRequest.APIFormat(), + PullRequest: issue.PullRequest.apiFormat(sess), Repository: issue.Repo.innerAPIFormat(sess, mode, false), Sender: doer.APIFormat(), } diff --git a/models/issue_mail.go b/models/issue_mail.go index 7f780e667..78dbd13de 100644 --- a/models/issue_mail.go +++ b/models/issue_mail.go @@ -48,7 +48,7 @@ func mailIssueCommentToParticipants(e Engine, issue *Issue, doer *User, content } // Assignees must receive any communications - assignees, err := GetAssigneesByIssue(issue) + assignees, err := getAssigneesByIssue(e, issue) if err != nil { return err } diff --git a/models/issue_user.go b/models/issue_user.go index 733c35df1..58eb5117f 100644 --- a/models/issue_user.go +++ b/models/issue_user.go @@ -54,7 +54,7 @@ func newIssueUsers(e Engine, repo *Repository, issue *Issue) error { func updateIssueAssignee(e *xorm.Session, issue *Issue, assigneeID int64) (removed bool, err error) { // Check if the user exists - assignee, err := GetUserByID(assigneeID) + assignee, err := getUserByID(e, assigneeID) if err != nil { return false, err } diff --git a/models/org.go b/models/org.go index bba5e22a2..81542f0e6 100644 --- a/models/org.go +++ b/models/org.go @@ -393,8 +393,12 @@ func GetOrgUsersByUserID(uid int64, all bool) ([]*OrgUser, error) { // GetOrgUsersByOrgID returns all organization-user relations by organization ID. func GetOrgUsersByOrgID(orgID int64) ([]*OrgUser, error) { + return getOrgUsersByOrgID(x, orgID) +} + +func getOrgUsersByOrgID(e Engine, orgID int64) ([]*OrgUser, error) { ous := make([]*OrgUser, 0, 10) - err := x. + err := e. Where("org_id=?", orgID). Find(&ous) return ous, err diff --git a/models/repo_watch.go b/models/repo_watch.go index 95c7e44e9..53a34efda 100644 --- a/models/repo_watch.go +++ b/models/repo_watch.go @@ -113,15 +113,15 @@ func notifyWatchers(e Engine, act *Action) error { switch act.OpType { case ActionCommitRepo, ActionPushTag, ActionDeleteTag, ActionDeleteBranch: - if !act.Repo.CheckUnitUser(act.UserID, false, UnitTypeCode) { + if !act.Repo.checkUnitUser(e, act.UserID, false, UnitTypeCode) { continue } case ActionCreateIssue, ActionCommentIssue, ActionCloseIssue, ActionReopenIssue: - if !act.Repo.CheckUnitUser(act.UserID, false, UnitTypeIssues) { + if !act.Repo.checkUnitUser(e, act.UserID, false, UnitTypeIssues) { continue } case ActionCreatePullRequest, ActionMergePullRequest, ActionClosePullRequest, ActionReopenPullRequest: - if !act.Repo.CheckUnitUser(act.UserID, false, UnitTypePullRequests) { + if !act.Repo.checkUnitUser(e, act.UserID, false, UnitTypePullRequests) { continue } }