Skip to content

Commit a291a0e

Browse files
authored
Prevent deadlock in pull_service.GetDiverging(pr) (#10905)
* Switch to use a temporary repository instead of adding remotes to the base gitea repository to prevent deadlocking the base gitea repository. * Add documentation on how to use func **createTemporaryRepo**
1 parent d501aec commit a291a0e

File tree

2 files changed

+10
-47
lines changed

2 files changed

+10
-47
lines changed

services/pull/temp_repo.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616
"code.gitea.io/gitea/modules/log"
1717
)
1818

19+
// createTemporaryRepo creates a temporary repo with "base" for pr.BaseBranch and "tracking" for pr.HeadBranch
20+
// it also create a second base branch called "original_base"
1921
func createTemporaryRepo(pr *models.PullRequest) (string, error) {
2022
if err := pr.LoadHeadRepo(); err != nil {
2123
log.Error("LoadHeadRepo: %v", err)

services/pull/update.go

Lines changed: 8 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ package pull
66

77
import (
88
"fmt"
9-
"strconv"
10-
"strings"
119

1210
"code.gitea.io/gitea/models"
1311
"code.gitea.io/gitea/modules/git"
@@ -66,62 +64,25 @@ func IsUserAllowedToUpdate(pull *models.PullRequest, user *models.User) (bool, e
6664

6765
// GetDiverging determines how many commits a PR is ahead or behind the PR base branch
6866
func GetDiverging(pr *models.PullRequest) (*git.DivergeObject, error) {
69-
log.Trace("PushToBaseRepo[%d]: pushing commits to base repo '%s'", pr.BaseRepoID, pr.GetGitRefName())
67+
log.Trace("GetDiverging[%d]: compare commits", pr.ID)
7068
if err := pr.LoadBaseRepo(); err != nil {
7169
return nil, err
7270
}
7371
if err := pr.LoadHeadRepo(); err != nil {
7472
return nil, err
7573
}
7674

77-
headRepoPath := pr.HeadRepo.RepoPath()
78-
headGitRepo, err := git.OpenRepository(headRepoPath)
75+
tmpRepo, err := createTemporaryRepo(pr)
7976
if err != nil {
80-
return nil, fmt.Errorf("OpenRepository: %v", err)
81-
}
82-
defer headGitRepo.Close()
83-
84-
if pr.IsSameRepo() {
85-
diff, err := git.GetDivergingCommits(pr.HeadRepo.RepoPath(), pr.BaseBranch, pr.HeadBranch)
86-
return &diff, err
87-
}
88-
89-
tmpRemoteName := fmt.Sprintf("tmp-pull-%d-base", pr.ID)
90-
if err = headGitRepo.AddRemote(tmpRemoteName, pr.BaseRepo.RepoPath(), true); err != nil {
91-
return nil, fmt.Errorf("headGitRepo.AddRemote: %v", err)
77+
log.Error("CreateTemporaryPath: %v", err)
78+
return nil, err
9279
}
93-
// Make sure to remove the remote even if the push fails
9480
defer func() {
95-
if err := headGitRepo.RemoveRemote(tmpRemoteName); err != nil {
96-
log.Error("CountDiverging: RemoveRemote: %s", err)
81+
if err := models.RemoveTemporaryPath(tmpRepo); err != nil {
82+
log.Error("Merge: RemoveTemporaryPath: %s", err)
9783
}
9884
}()
9985

100-
// $(git rev-list --count tmp-pull-1-base/master..feature) commits ahead of master
101-
ahead, errorAhead := checkDivergence(headRepoPath, fmt.Sprintf("%s/%s", tmpRemoteName, pr.BaseBranch), pr.HeadBranch)
102-
if errorAhead != nil {
103-
return &git.DivergeObject{}, errorAhead
104-
}
105-
106-
// $(git rev-list --count feature..tmp-pull-1-base/master) commits behind master
107-
behind, errorBehind := checkDivergence(headRepoPath, pr.HeadBranch, fmt.Sprintf("%s/%s", tmpRemoteName, pr.BaseBranch))
108-
if errorBehind != nil {
109-
return &git.DivergeObject{}, errorBehind
110-
}
111-
112-
return &git.DivergeObject{Ahead: ahead, Behind: behind}, nil
113-
}
114-
115-
func checkDivergence(repoPath string, baseBranch string, targetBranch string) (int, error) {
116-
branches := fmt.Sprintf("%s..%s", baseBranch, targetBranch)
117-
cmd := git.NewCommand("rev-list", "--count", branches)
118-
stdout, err := cmd.RunInDir(repoPath)
119-
if err != nil {
120-
return -1, err
121-
}
122-
outInteger, errInteger := strconv.Atoi(strings.Trim(stdout, "\n"))
123-
if errInteger != nil {
124-
return -1, errInteger
125-
}
126-
return outInteger, nil
86+
diff, err := git.GetDivergingCommits(tmpRepo, "base", "tracking")
87+
return &diff, err
12788
}

0 commit comments

Comments
 (0)