From e9f6a4307372b8e82518b10c89a9a7d649153d73 Mon Sep 17 00:00:00 2001
From: stroucki <stroucki@andrew.cmu.edu>
Date: Wed, 21 Dec 2016 04:11:07 -0500
Subject: [PATCH] Fix database write context interleaving bug (#3822)

* UpdateIssueUsersByMentions was calling database write operations while
a transaction session was in progress. MailParticipants was failing
silently because of the SQLITE_LOCKED error. Make sure failures in
MailParticipants enter the log, and pass on the transaction context.

issue: let caller pass in database context, and use it
issue_comment: obtain database context to pass to UpdateIssueMentions
issue_comment: log any error from call to MailParticipants
issue_mail: pass on database context to UpdateIssueMentions

* issue: forgot debug statement
---
 models/issue.go         | 14 +++++++-------
 models/issue_comment.go |  8 +++++---
 models/issue_mail.go    |  2 +-
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/models/issue.go b/models/issue.go
index 6dfb47175..e14b44622 100644
--- a/models/issue.go
+++ b/models/issue.go
@@ -1026,7 +1026,7 @@ func GetIssueUserPairsByMode(uid, rid int64, isClosed bool, page, filterMode int
 
 // UpdateIssueMentions extracts mentioned people from content and
 // updates issue-user relations for them.
-func UpdateIssueMentions(issueID int64, mentions []string) error {
+func UpdateIssueMentions(e Engine, issueID int64, mentions []string) error {
 	if len(mentions) == 0 {
 		return nil
 	}
@@ -1036,7 +1036,7 @@ func UpdateIssueMentions(issueID int64, mentions []string) error {
 	}
 	users := make([]*User, 0, len(mentions))
 
-	if err := x.In("lower_name", mentions).Asc("lower_name").Find(&users); err != nil {
+	if err := e.In("lower_name", mentions).Asc("lower_name").Find(&users); err != nil {
 		return fmt.Errorf("find mentioned users: %v", err)
 	}
 
@@ -1060,7 +1060,7 @@ func UpdateIssueMentions(issueID int64, mentions []string) error {
 		ids = append(ids, memberIDs...)
 	}
 
-	if err := UpdateIssueUsersByMentions(issueID, ids); err != nil {
+	if err := UpdateIssueUsersByMentions(e, issueID, ids); err != nil {
 		return fmt.Errorf("UpdateIssueUsersByMentions: %v", err)
 	}
 
@@ -1293,22 +1293,22 @@ func UpdateIssueUserByRead(uid, issueID int64) error {
 }
 
 // UpdateIssueUsersByMentions updates issue-user pairs by mentioning.
-func UpdateIssueUsersByMentions(issueID int64, uids []int64) error {
+func UpdateIssueUsersByMentions(e Engine, issueID int64, uids []int64) error {
 	for _, uid := range uids {
 		iu := &IssueUser{
 			UID:     uid,
 			IssueID: issueID,
 		}
-		has, err := x.Get(iu)
+		has, err := e.Get(iu)
 		if err != nil {
 			return err
 		}
 
 		iu.IsMentioned = true
 		if has {
-			_, err = x.Id(iu.ID).AllCols().Update(iu)
+			_, err = e.Id(iu.ID).AllCols().Update(iu)
 		} else {
-			_, err = x.Insert(iu)
+			_, err = e.Insert(iu)
 		}
 		if err != nil {
 			return err
diff --git a/models/issue_comment.go b/models/issue_comment.go
index 091bfc3b2..3f81889c4 100644
--- a/models/issue_comment.go
+++ b/models/issue_comment.go
@@ -162,9 +162,9 @@ func (c *Comment) EventTag() string {
 
 // MailParticipants sends new comment emails to repository watchers
 // and mentioned people.
-func (cmt *Comment) MailParticipants(opType ActionType, issue *Issue) (err error) {
+func (cmt *Comment) MailParticipants(e Engine, opType ActionType, issue *Issue) (err error) {
 	mentions := markdown.FindAllMentions(cmt.Content)
-	if err = UpdateIssueMentions(cmt.IssueID, mentions); err != nil {
+	if err = UpdateIssueMentions(e, cmt.IssueID, mentions); err != nil {
 		return fmt.Errorf("UpdateIssueMentions [%d]: %v", cmt.IssueID, err)
 	}
 
@@ -278,7 +278,9 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
 		if err = notifyWatchers(e, act); err != nil {
 			log.Error(4, "notifyWatchers: %v", err)
 		}
-		comment.MailParticipants(act.OpType, opts.Issue)
+		if err = comment.MailParticipants(e, act.OpType, opts.Issue); err != nil {
+			log.Error(4, "MailParticipants: %v", err)
+		}
 	}
 
 	return comment, comment.loadAttributes(e)
diff --git a/models/issue_mail.go b/models/issue_mail.go
index 02548bc3e..c02255724 100644
--- a/models/issue_mail.go
+++ b/models/issue_mail.go
@@ -69,7 +69,7 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string)
 // and mentioned people.
 func (issue *Issue) MailParticipants() (err error) {
 	mentions := markdown.FindAllMentions(issue.Content)
-	if err = UpdateIssueMentions(issue.ID, mentions); err != nil {
+	if err = UpdateIssueMentions(x, issue.ID, mentions); err != nil {
 		return fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err)
 	}