Skip to content

Commit 3497efa

Browse files
zeripathzeripath
authored
Add Close() method to gogitRepository (#8901) (#8956)
Backport #8901 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 43fc99a commit 3497efa

Some content is hidden

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

72 files changed

+386
-102
lines changed

‎cmd/admin.go

+3
Original file line numberDiff line numberDiff line change
@@ -375,17 +375,20 @@ func runRepoSyncReleases(c *cli.Context) error {
375375

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

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

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

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ type Uploader interface {
6868
CreateComment(issueNumber int64, comment *Comment) error
6969
CreatePullRequest(pr *PullRequest) error
7070
Rollback() error
71+
Close()
7172
}
7273

73-
```
74+
```

‎integrations/api_releases_test.go

+2
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

+1
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

+1
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

+2
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

+2
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

+2
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

+5
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

+18
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/graph_test.go

+1
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/migrations/v39.go

+1
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

+1
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

+5
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ func (pr *PullRequest) GetLastCommitStatus() (status *CommitStatus, err error) {
382382
if err != nil {
383383
return nil, err
384384
}
385+
defer headGitRepo.Close()
385386

386387
lastCommitID, err := headGitRepo.GetBranchCommitID(pr.HeadBranch)
387388
if err != nil {
@@ -571,6 +572,7 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) {
571572
if err != nil {
572573
return nil, fmt.Errorf("OpenRepository: %v", err)
573574
}
575+
defer gitRepo.Close()
574576

575577
commit, err := gitRepo.GetCommit(mergeCommit[:40])
576578
if err != nil {
@@ -955,6 +957,7 @@ func (pr *PullRequest) UpdatePatch() (err error) {
955957
if err != nil {
956958
return fmt.Errorf("OpenRepository: %v", err)
957959
}
960+
defer headGitRepo.Close()
958961

959962
// Add a temporary remote.
960963
tmpRemote := com.ToStr(time.Now().UnixNano())
@@ -996,6 +999,7 @@ func (pr *PullRequest) PushToBaseRepo() (err error) {
996999
if err != nil {
9971000
return fmt.Errorf("OpenRepository: %v", err)
9981001
}
1002+
defer headGitRepo.Close()
9991003

10001004
tmpRemoteName := fmt.Sprintf("tmp-pull-%d", pr.ID)
10011005
if err = headGitRepo.AddRemote(tmpRemoteName, pr.BaseRepo.RepoPath(), false); err != nil {
@@ -1185,6 +1189,7 @@ func checkForInvalidation(requests PullRequestList, repoID int64, doer *User, br
11851189
if err != nil {
11861190
log.Error("PullRequestList.InvalidateCodeComments: %v", err)
11871191
}
1192+
gitRepo.Close()
11881193
}()
11891194
return nil
11901195
}

‎models/repo.go

+1
Original file line numberDiff line numberDiff line change
@@ -998,6 +998,7 @@ func MigrateRepositoryGitData(doer, u *User, repo *Repository, opts api.MigrateR
998998
if err != nil {
999999
return repo, fmt.Errorf("OpenRepository: %v", err)
10001000
}
1001+
defer gitRepo.Close()
10011002

10021003
repo.IsEmpty, err = gitRepo.IsEmpty()
10031004
if err != nil {

‎models/repo_activity.go

+4
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

+4
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)

‎models/repo_tag.go

-24
This file was deleted.

‎models/wiki.go

+2
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ func (repo *Repository) updateWikiPage(doer *User, oldWikiName, newWikiName, con
140140
log.Error("Unable to open temporary repository: %s (%v)", basePath, err)
141141
return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err)
142142
}
143+
defer gitRepo.Close()
143144

144145
if hasMasterBranch {
145146
if err := gitRepo.ReadTreeToIndex("HEAD"); err != nil {
@@ -276,6 +277,7 @@ func (repo *Repository) DeleteWikiPage(doer *User, wikiName string) (err error)
276277
log.Error("Unable to open temporary repository: %s (%v)", basePath, err)
277278
return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err)
278279
}
280+
defer gitRepo.Close()
279281

280282
if err := gitRepo.ReadTreeToIndex("HEAD"); err != nil {
281283
log.Error("Unable to read HEAD tree to index in: %s %v", basePath, err)

‎models/wiki_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ func TestRepository_AddWikiPage(t *testing.T) {
161161
// Now need to show that the page has been added:
162162
gitRepo, err := git.OpenRepository(repo.WikiPath())
163163
assert.NoError(t, err)
164+
defer gitRepo.Close()
164165
masterTree, err := gitRepo.GetTree("master")
165166
assert.NoError(t, err)
166167
wikiPath := WikiNameToFilename(wikiName)
@@ -214,6 +215,7 @@ func TestRepository_EditWikiPage(t *testing.T) {
214215
_, err := masterTree.GetTreeEntryByPath("Home.md")
215216
assert.Error(t, err)
216217
}
218+
gitRepo.Close()
217219
}
218220
}
219221

@@ -226,6 +228,7 @@ func TestRepository_DeleteWikiPage(t *testing.T) {
226228
// Now need to show that the page has been added:
227229
gitRepo, err := git.OpenRepository(repo.WikiPath())
228230
assert.NoError(t, err)
231+
defer gitRepo.Close()
229232
masterTree, err := gitRepo.GetTree("master")
230233
assert.NoError(t, err)
231234
wikiPath := WikiNameToFilename("Home")

0 commit comments

Comments
 (0)