Skip to content

Commit

Permalink
Fix sqlite deadlock when assigning to a PR (#5640)
Browse files Browse the repository at this point in the history
* Fix sqlite deadlock when assigning to a PR

Fix 5639

Signed-off-by: Andrew Thornton <art27@cantab.net>

* More possible deadlocks found and fixed

Signed-off-by: Andrew Thornton <art27@cantab.net>
  • Loading branch information
zeripath authored and techknowlogick committed Jan 4, 2019
1 parent 9e90103 commit 6311e4c
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 9 deletions.
2 changes: 1 addition & 1 deletion models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
8 changes: 6 additions & 2 deletions models/issue_assignees.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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(),
}
Expand Down
2 changes: 1 addition & 1 deletion models/issue_mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion models/issue_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 5 additions & 1 deletion models/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions models/repo_watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down

0 comments on commit 6311e4c

Please sign in to comment.