Skip to content

Commit fb5af37

Browse files
zeripathzeripath
authored
Add Close() method to gogitRepository (#8901) (#8958)
Backport #8901 - Adjusted slightly for 1.9 In investigating #7947 it has become clear that the storage component of go-git repositories needs closing. This PR adds this Close function and adds the Close functions as necessary. In TransferOwnership the ctx.Repo.GitRepo is closed if it is open to help prevent the risk of multiple open files. Fixes #7947
1 parent 2ef3752 commit fb5af37

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

71 files changed

+275
-31
lines changed

‎cmd/admin.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,17 +374,20 @@ func runRepoSyncReleases(c *cli.Context) error {
374374

375375
if err = models.SyncReleasesWithTags(repo, gitRepo); err != nil {
376376
log.Warn(" SyncReleasesWithTags: %v", err)
377+
gitRepo.Close()
377378
continue
378379
}
379380

380381
count, err = getReleaseCount(repo.ID)
381382
if err != nil {
382383
log.Warn(" GetReleaseCountByRepoID: %v", err)
384+
gitRepo.Close()
383385
continue
384386
}
385387

386388
log.Trace(" repo %s releases synchronized to tags: from %d to %d",
387389
repo.FullName(), oldnum, count)
390+
gitRepo.Close()
388391
}
389392
}
390393

‎docs/content/doc/advanced/migrations.en-us.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ type Uploader interface {
6767
CreateComment(issueNumber int64, comment *Comment) error
6868
CreatePullRequest(pr *PullRequest) error
6969
Rollback() error
70+
Close()
7071
}
7172

72-
```
73+
```

‎go.mod

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,6 @@ require (
8383
github.com/mattn/go-sqlite3 v1.11.0
8484
github.com/mcuadros/go-version v0.0.0-20190308113854-92cdf37c5b75
8585
github.com/microcosm-cc/bluemonday v0.0.0-20161012083705-f77f16ffc87a
86-
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
87-
github.com/modern-go/reflect2 v1.0.1 // indirect
8886
github.com/mschoch/smat v0.0.0-20160514031455-90eadee771ae // indirect
8987
github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc
9088
github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5

‎integrations/api_releases_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ func TestAPICreateAndUpdateRelease(t *testing.T) {
5151

5252
gitRepo, err := git.OpenRepository(repo.RepoPath())
5353
assert.NoError(t, err)
54+
defer gitRepo.Close()
5455

5556
err = gitRepo.CreateTag("v0.0.1", "master")
5657
assert.NoError(t, err)
@@ -112,6 +113,7 @@ func TestAPICreateReleaseToDefaultBranchOnExistingTag(t *testing.T) {
112113

113114
gitRepo, err := git.OpenRepository(repo.RepoPath())
114115
assert.NoError(t, err)
116+
defer gitRepo.Close()
115117

116118
err = gitRepo.CreateTag("v0.0.1", "master")
117119
assert.NoError(t, err)

‎integrations/api_repo_file_create_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ func TestAPICreateFile(t *testing.T) {
139139
assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, fileResponse.Commit.HTMLURL)
140140
assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, fileResponse.Commit.Author.Email)
141141
assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, fileResponse.Commit.Author.Name)
142+
gitRepo.Close()
142143
}
143144

144145
// Test creating a file in a new branch

‎integrations/api_repo_file_update_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ func TestAPIUpdateFile(t *testing.T) {
143143
assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, fileResponse.Commit.HTMLURL)
144144
assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, fileResponse.Commit.Author.Email)
145145
assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, fileResponse.Commit.Author.Name)
146+
gitRepo.Close()
146147
}
147148

148149
// Test updating a file in a new branch

‎integrations/api_repo_get_contents_list_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) {
7474
repo1.CreateNewBranch(user2, repo1.DefaultBranch, newBranch)
7575
// Get the commit ID of the default branch
7676
gitRepo, _ := git.OpenRepository(repo1.RepoPath())
77+
defer gitRepo.Close()
78+
7779
commitID, _ := gitRepo.GetBranchCommitID(repo1.DefaultBranch)
7880
// Make a new tag in repo1
7981
newTag := "test_tag"

‎integrations/api_repo_get_contents_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ func testAPIGetContents(t *testing.T, u *url.URL) {
7575
repo1.CreateNewBranch(user2, repo1.DefaultBranch, newBranch)
7676
// Get the commit ID of the default branch
7777
gitRepo, _ := git.OpenRepository(repo1.RepoPath())
78+
defer gitRepo.Close()
79+
7880
commitID, _ := gitRepo.GetBranchCommitID(repo1.DefaultBranch)
7981
// Make a new tag in repo1
8082
newTag := "test_tag"

‎integrations/api_repo_git_tags_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ func TestAPIGitTags(t *testing.T) {
2929
git.NewCommand("config", "user.email", user.Email).RunInDir(repo.RepoPath())
3030

3131
gitRepo, _ := git.OpenRepository(repo.RepoPath())
32+
defer gitRepo.Close()
33+
3234
commit, _ := gitRepo.GetBranchCommit("master")
3335
lTagName := "lightweightTag"
3436
gitRepo.CreateTag(lTagName, commit.ID.String())

‎integrations/repofiles_delete_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ func testDeleteRepoFile(t *testing.T, u *url.URL) {
7373
test.LoadRepoCommit(t, ctx)
7474
test.LoadUser(t, ctx, 2)
7575
test.LoadGitRepo(t, ctx)
76+
defer ctx.Repo.GitRepo.Close()
7677
repo := ctx.Repo.Repository
7778
doer := ctx.User
7879
opts := getDeleteRepoFileOptions(repo)
@@ -111,6 +112,8 @@ func testDeleteRepoFileWithoutBranchNames(t *testing.T, u *url.URL) {
111112
test.LoadRepoCommit(t, ctx)
112113
test.LoadUser(t, ctx, 2)
113114
test.LoadGitRepo(t, ctx)
115+
defer ctx.Repo.GitRepo.Close()
116+
114117
repo := ctx.Repo.Repository
115118
doer := ctx.User
116119
opts := getDeleteRepoFileOptions(repo)
@@ -139,6 +142,8 @@ func TestDeleteRepoFileErrors(t *testing.T) {
139142
test.LoadRepoCommit(t, ctx)
140143
test.LoadUser(t, ctx, 2)
141144
test.LoadGitRepo(t, ctx)
145+
defer ctx.Repo.GitRepo.Close()
146+
142147
repo := ctx.Repo.Repository
143148
doer := ctx.User
144149

‎integrations/repofiles_update_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ func TestCreateOrUpdateRepoFileForCreate(t *testing.T) {
191191
test.LoadRepoCommit(t, ctx)
192192
test.LoadUser(t, ctx, 2)
193193
test.LoadGitRepo(t, ctx)
194+
defer ctx.Repo.GitRepo.Close()
195+
194196
repo := ctx.Repo.Repository
195197
doer := ctx.User
196198
opts := getCreateRepoFileOptions(repo)
@@ -201,6 +203,8 @@ func TestCreateOrUpdateRepoFileForCreate(t *testing.T) {
201203
// asserts
202204
assert.Nil(t, err)
203205
gitRepo, _ := git.OpenRepository(repo.RepoPath())
206+
defer gitRepo.Close()
207+
204208
commitID, _ := gitRepo.GetBranchCommitID(opts.NewBranch)
205209
expectedFileResponse := getExpectedFileResponseForRepofilesCreate(commitID)
206210
assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content)
@@ -220,6 +224,8 @@ func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) {
220224
test.LoadRepoCommit(t, ctx)
221225
test.LoadUser(t, ctx, 2)
222226
test.LoadGitRepo(t, ctx)
227+
defer ctx.Repo.GitRepo.Close()
228+
223229
repo := ctx.Repo.Repository
224230
doer := ctx.User
225231
opts := getUpdateRepoFileOptions(repo)
@@ -230,6 +236,8 @@ func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) {
230236
// asserts
231237
assert.Nil(t, err)
232238
gitRepo, _ := git.OpenRepository(repo.RepoPath())
239+
defer gitRepo.Close()
240+
233241
commitID, _ := gitRepo.GetBranchCommitID(opts.NewBranch)
234242
expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commitID, opts.TreePath)
235243
assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content)
@@ -249,6 +257,8 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) {
249257
test.LoadRepoCommit(t, ctx)
250258
test.LoadUser(t, ctx, 2)
251259
test.LoadGitRepo(t, ctx)
260+
defer ctx.Repo.GitRepo.Close()
261+
252262
repo := ctx.Repo.Repository
253263
doer := ctx.User
254264
opts := getUpdateRepoFileOptions(repo)
@@ -261,6 +271,8 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) {
261271
// asserts
262272
assert.Nil(t, err)
263273
gitRepo, _ := git.OpenRepository(repo.RepoPath())
274+
defer gitRepo.Close()
275+
264276
commit, _ := gitRepo.GetBranchCommit(opts.NewBranch)
265277
expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.TreePath)
266278
// assert that the old file no longer exists in the last commit of the branch
@@ -288,6 +300,8 @@ func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) {
288300
test.LoadRepoCommit(t, ctx)
289301
test.LoadUser(t, ctx, 2)
290302
test.LoadGitRepo(t, ctx)
303+
defer ctx.Repo.GitRepo.Close()
304+
291305
repo := ctx.Repo.Repository
292306
doer := ctx.User
293307
opts := getUpdateRepoFileOptions(repo)
@@ -300,6 +314,8 @@ func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) {
300314
// asserts
301315
assert.Nil(t, err)
302316
gitRepo, _ := git.OpenRepository(repo.RepoPath())
317+
defer gitRepo.Close()
318+
303319
commitID, _ := gitRepo.GetBranchCommitID(repo.DefaultBranch)
304320
expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commitID, opts.TreePath)
305321
assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content)
@@ -315,6 +331,8 @@ func TestCreateOrUpdateRepoFileErrors(t *testing.T) {
315331
test.LoadRepoCommit(t, ctx)
316332
test.LoadUser(t, ctx, 2)
317333
test.LoadGitRepo(t, ctx)
334+
defer ctx.Repo.GitRepo.Close()
335+
318336
repo := ctx.Repo.Repository
319337
doer := ctx.User
320338

‎models/action.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -762,6 +762,7 @@ func CommitRepoAction(opts CommitRepoActionOptions) error {
762762
if err != nil {
763763
log.Error("GetBranchCommitID[%s]: %v", opts.RefFullName, err)
764764
}
765+
gitRepo.Close()
765766
if err = PrepareWebhooks(repo, HookEventCreate, &api.CreatePayload{
766767
Ref: refName,
767768
Sha: shaSum,
@@ -797,6 +798,8 @@ func CommitRepoAction(opts CommitRepoActionOptions) error {
797798
if err != nil {
798799
log.Error("GetTagCommitID[%s]: %v", opts.RefFullName, err)
799800
}
801+
gitRepo.Close()
802+
800803
if err = PrepareWebhooks(repo, HookEventCreate, &api.CreatePayload{
801804
Ref: refName,
802805
Sha: shaSum,

‎models/git_diff.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID
675675
if err != nil {
676676
return nil, err
677677
}
678+
defer gitRepo.Close()
678679

679680
commit, err := gitRepo.GetCommit(afterCommitID)
680681
if err != nil {
@@ -747,6 +748,7 @@ func GetRawDiffForFile(repoPath, startCommit, endCommit string, diffType RawDiff
747748
if err != nil {
748749
return fmt.Errorf("OpenRepository: %v", err)
749750
}
751+
defer repo.Close()
750752

751753
commit, err := repo.GetCommit(endCommit)
752754
if err != nil {

‎models/graph_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ func BenchmarkGetCommitGraph(b *testing.B) {
1717
if err != nil {
1818
b.Error("Could not open repository")
1919
}
20+
defer currentRepo.Close()
2021

2122
for i := 0; i < b.N; i++ {
2223
graph, err := GetCommitGraph(currentRepo)

‎models/issue_comment.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,7 @@ func CreateCodeComment(doer *User, repo *Repository, issue *Issue, content, tree
893893
if err != nil {
894894
return nil, fmt.Errorf("OpenRepository: %v", err)
895895
}
896+
defer gitRepo.Close()
896897

897898
// FIXME validate treePath
898899
// Get latest commit referencing the commented line

‎models/migrations/v39.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ func releaseAddColumnIsTagAndSyncTags(x *xorm.Engine) error {
4747
if err = models.SyncReleasesWithTags(repo, gitRepo); err != nil {
4848
log.Warn("SyncReleasesWithTags: %v", err)
4949
}
50+
gitRepo.Close()
5051
}
5152
if len(repos) < pageSize {
5253
break

‎models/migrations/v82.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ func fixReleaseSha1OnReleaseTable(x *xorm.Engine) error {
9191
if err != nil {
9292
return err
9393
}
94+
defer gitRepo.Close()
9495
gitRepoCache[release.RepoID] = gitRepo
9596
}
9697

‎models/pull.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ func (pr *PullRequest) GetLastCommitStatus() (status *CommitStatus, err error) {
338338
if err != nil {
339339
return nil, err
340340
}
341+
defer headGitRepo.Close()
341342

342343
lastCommitID, err := headGitRepo.GetBranchCommitID(pr.HeadBranch)
343344
if err != nil {
@@ -527,6 +528,7 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) {
527528
if err != nil {
528529
return nil, fmt.Errorf("OpenRepository: %v", err)
529530
}
531+
defer gitRepo.Close()
530532

531533
commit, err := gitRepo.GetCommit(mergeCommit[:40])
532534
if err != nil {
@@ -920,6 +922,7 @@ func (pr *PullRequest) UpdatePatch() (err error) {
920922
if err != nil {
921923
return fmt.Errorf("OpenRepository: %v", err)
922924
}
925+
defer headGitRepo.Close()
923926

924927
// Add a temporary remote.
925928
tmpRemote := com.ToStr(time.Now().UnixNano())
@@ -961,6 +964,7 @@ func (pr *PullRequest) PushToBaseRepo() (err error) {
961964
if err != nil {
962965
return fmt.Errorf("OpenRepository: %v", err)
963966
}
967+
defer headGitRepo.Close()
964968

965969
tmpRemoteName := fmt.Sprintf("tmp-pull-%d", pr.ID)
966970
if err = headGitRepo.AddRemote(tmpRemoteName, pr.BaseRepo.RepoPath(), false); err != nil {
@@ -1150,6 +1154,7 @@ func checkForInvalidation(requests PullRequestList, repoID int64, doer *User, br
11501154
if err != nil {
11511155
log.Error("PullRequestList.InvalidateCodeComments: %v", err)
11521156
}
1157+
gitRepo.Close()
11531158
}()
11541159
return nil
11551160
}

‎models/release_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ func TestRelease_Create(t *testing.T) {
2121

2222
gitRepo, err := git.OpenRepository(repoPath)
2323
assert.NoError(t, err)
24+
defer gitRepo.Close()
2425

2526
assert.NoError(t, CreateRelease(gitRepo, &Release{
2627
RepoID: repo.ID,
@@ -115,6 +116,7 @@ func TestRelease_MirrorDelete(t *testing.T) {
115116

116117
gitRepo, err := git.OpenRepository(repoPath)
117118
assert.NoError(t, err)
119+
defer gitRepo.Close()
118120

119121
findOptions := FindReleasesOptions{IncludeDrafts: true, IncludeTags: true}
120122
initCount, err := GetReleaseCountByRepoID(mirror.ID, findOptions)

‎models/repo.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,7 @@ func MigrateRepository(doer, u *User, opts MigrateRepoOptions) (*Repository, err
950950
if err != nil {
951951
return repo, fmt.Errorf("OpenRepository: %v", err)
952952
}
953+
defer gitRepo.Close()
953954

954955
repo.IsEmpty, err = gitRepo.IsEmpty()
955956
if err != nil {

‎models/repo_activity.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ func GetActivityStats(repo *Repository, timeFrom time.Time, releases, issues, pr
6464
if err != nil {
6565
return nil, fmt.Errorf("OpenRepository: %v", err)
6666
}
67+
defer gitRepo.Close()
68+
6769
code, err := gitRepo.GetCodeActivityStats(timeFrom, repo.DefaultBranch)
6870
if err != nil {
6971
return nil, fmt.Errorf("FillFromGit: %v", err)
@@ -79,6 +81,8 @@ func GetActivityStatsTopAuthors(repo *Repository, timeFrom time.Time, count int)
7981
if err != nil {
8082
return nil, fmt.Errorf("OpenRepository: %v", err)
8183
}
84+
defer gitRepo.Close()
85+
8286
code, err := gitRepo.GetCodeActivityStats(timeFrom, "")
8387
if err != nil {
8488
return nil, fmt.Errorf("FillFromGit: %v", err)

‎models/repo_branch.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ func (repo *Repository) GetBranch(branch string) (*git.Branch, error) {
2323
if err != nil {
2424
return nil, err
2525
}
26+
defer gitRepo.Close()
2627

2728
return gitRepo.GetBranch(branch)
2829
}
@@ -38,6 +39,7 @@ func (repo *Repository) CheckBranchName(name string) error {
3839
if err != nil {
3940
return err
4041
}
42+
defer gitRepo.Close()
4143

4244
branches, err := repo.GetBranches()
4345
if err != nil {
@@ -94,6 +96,7 @@ func (repo *Repository) CreateNewBranch(doer *User, oldBranchName, branchName st
9496
log.Error("Unable to open temporary repository: %s (%v)", basePath, err)
9597
return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err)
9698
}
99+
defer gitRepo.Close()
97100

98101
if err = gitRepo.CreateBranch(branchName, oldBranchName); err != nil {
99102
log.Error("Unable to create branch: %s from %s. (%v)", branchName, oldBranchName, err)
@@ -140,6 +143,7 @@ func (repo *Repository) CreateNewBranchFromCommit(doer *User, commit, branchName
140143
log.Error("Unable to open temporary repository: %s (%v)", basePath, err)
141144
return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err)
142145
}
146+
defer gitRepo.Close()
143147

144148
if err = gitRepo.CreateBranch(branchName, commit); err != nil {
145149
log.Error("Unable to create branch: %s from %s. (%v)", branchName, commit, err)

0 commit comments

Comments
 (0)