From fe3f28712a373c971692d55318f5905f68388395 Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 13 Nov 2019 07:01:19 +0000 Subject: [PATCH] Add Close() method to gogitRepository (#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 --- cmd/admin.go | 3 ++ docs/content/doc/advanced/migrations.en-us.md | 3 +- go.mod | 2 - integrations/api_releases_test.go | 2 + integrations/api_repo_file_create_test.go | 1 + integrations/api_repo_file_update_test.go | 1 + .../api_repo_get_contents_list_test.go | 2 + integrations/api_repo_get_contents_test.go | 2 + integrations/api_repo_git_tags_test.go | 2 + integrations/repofiles_delete_test.go | 5 +++ integrations/repofiles_update_test.go | 18 +++++++++ models/action.go | 3 ++ models/git_diff.go | 2 + models/graph_test.go | 1 + models/issue_comment.go | 1 + models/migrations/v39.go | 1 + models/migrations/v82.go | 1 + models/pull.go | 5 +++ models/release_test.go | 2 + models/repo.go | 1 + models/repo_activity.go | 4 ++ models/repo_branch.go | 4 ++ models/repo_mirror.go | 3 ++ models/repo_tag.go | 24 ------------ models/update.go | 1 + models/wiki.go | 2 + models/wiki_test.go | 3 ++ modules/context/api.go | 9 +++++ modules/context/repo.go | 18 +++++++++ modules/git/blame.go | 3 +- modules/git/blob_test.go | 4 ++ modules/git/commit_info_test.go | 10 ++++- modules/git/notes_test.go | 2 + modules/git/repo.go | 16 ++++++++ modules/git/repo_blob_test.go | 3 ++ modules/git/repo_branch.go | 1 + modules/git/repo_branch_test.go | 2 + modules/git/repo_commit_test.go | 7 ++++ modules/git/repo_compare_test.go | 1 + modules/git/repo_ref_test.go | 2 + modules/git/repo_stats_test.go | 1 + modules/git/repo_tag_test.go | 3 ++ modules/git/repo_test.go | 1 + modules/git/tree_entry_test.go | 1 + modules/migrations/base/uploader.go | 1 + modules/migrations/gitea.go | 7 ++++ modules/migrations/migrate.go | 1 + modules/repofiles/blob.go | 1 + modules/repofiles/blob_test.go | 2 + modules/repofiles/commit_status.go | 2 + modules/repofiles/content.go | 2 + modules/repofiles/content_test.go | 12 ++++++ modules/repofiles/diff_test.go | 4 ++ modules/repofiles/file_test.go | 3 ++ modules/repofiles/temp_repo.go | 1 + modules/repofiles/tree.go | 1 + modules/repofiles/tree_test.go | 2 + modules/test/context_tests.go | 1 + routers/api/v1/api.go | 2 +- routers/api/v1/repo/commits.go | 1 + routers/api/v1/repo/file.go | 1 + routers/api/v1/repo/git_ref.go | 2 + routers/api/v1/repo/pull.go | 7 ++++ routers/api/v1/repo/tag.go | 2 +- routers/repo/compare.go | 8 ++++ routers/repo/editor_test.go | 5 +++ routers/repo/pull.go | 6 +++ routers/repo/release_test.go | 1 + routers/repo/setting.go | 10 +++++ routers/repo/wiki.go | 37 +++++++++++++++++++ routers/repo/wiki_test.go | 1 + 71 files changed, 275 insertions(+), 31 deletions(-) delete mode 100644 models/repo_tag.go diff --git a/cmd/admin.go b/cmd/admin.go index 4c4d6f9b66e37..eae78d543f5dd 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -374,17 +374,20 @@ func runRepoSyncReleases(c *cli.Context) error { if err = models.SyncReleasesWithTags(repo, gitRepo); err != nil { log.Warn(" SyncReleasesWithTags: %v", err) + gitRepo.Close() continue } count, err = getReleaseCount(repo.ID) if err != nil { log.Warn(" GetReleaseCountByRepoID: %v", err) + gitRepo.Close() continue } log.Trace(" repo %s releases synchronized to tags: from %d to %d", repo.FullName(), oldnum, count) + gitRepo.Close() } } diff --git a/docs/content/doc/advanced/migrations.en-us.md b/docs/content/doc/advanced/migrations.en-us.md index 7db9cad817d1c..6a3021306b4e5 100644 --- a/docs/content/doc/advanced/migrations.en-us.md +++ b/docs/content/doc/advanced/migrations.en-us.md @@ -67,6 +67,7 @@ type Uploader interface { CreateComment(issueNumber int64, comment *Comment) error CreatePullRequest(pr *PullRequest) error Rollback() error + Close() } -``` \ No newline at end of file +``` diff --git a/go.mod b/go.mod index f7487d6e17442..3250fe8e95668 100644 --- a/go.mod +++ b/go.mod @@ -83,8 +83,6 @@ require ( github.com/mattn/go-sqlite3 v1.11.0 github.com/mcuadros/go-version v0.0.0-20190308113854-92cdf37c5b75 github.com/microcosm-cc/bluemonday v0.0.0-20161012083705-f77f16ffc87a - github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect - github.com/modern-go/reflect2 v1.0.1 // indirect github.com/mschoch/smat v0.0.0-20160514031455-90eadee771ae // indirect github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5 diff --git a/integrations/api_releases_test.go b/integrations/api_releases_test.go index 897f863eb3e03..8025f1de5d59b 100644 --- a/integrations/api_releases_test.go +++ b/integrations/api_releases_test.go @@ -51,6 +51,7 @@ func TestAPICreateAndUpdateRelease(t *testing.T) { gitRepo, err := git.OpenRepository(repo.RepoPath()) assert.NoError(t, err) + defer gitRepo.Close() err = gitRepo.CreateTag("v0.0.1", "master") assert.NoError(t, err) @@ -112,6 +113,7 @@ func TestAPICreateReleaseToDefaultBranchOnExistingTag(t *testing.T) { gitRepo, err := git.OpenRepository(repo.RepoPath()) assert.NoError(t, err) + defer gitRepo.Close() err = gitRepo.CreateTag("v0.0.1", "master") assert.NoError(t, err) diff --git a/integrations/api_repo_file_create_test.go b/integrations/api_repo_file_create_test.go index 42898bf259faa..948d3b6f1cc53 100644 --- a/integrations/api_repo_file_create_test.go +++ b/integrations/api_repo_file_create_test.go @@ -139,6 +139,7 @@ func TestAPICreateFile(t *testing.T) { assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, fileResponse.Commit.HTMLURL) assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, fileResponse.Commit.Author.Email) assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, fileResponse.Commit.Author.Name) + gitRepo.Close() } // Test creating a file in a new branch diff --git a/integrations/api_repo_file_update_test.go b/integrations/api_repo_file_update_test.go index 366eb5e91895e..c19d8d95829ce 100644 --- a/integrations/api_repo_file_update_test.go +++ b/integrations/api_repo_file_update_test.go @@ -143,6 +143,7 @@ func TestAPIUpdateFile(t *testing.T) { assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, fileResponse.Commit.HTMLURL) assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, fileResponse.Commit.Author.Email) assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, fileResponse.Commit.Author.Name) + gitRepo.Close() } // Test updating a file in a new branch diff --git a/integrations/api_repo_get_contents_list_test.go b/integrations/api_repo_get_contents_list_test.go index f74ceb514a0bc..4605ccf4d933b 100644 --- a/integrations/api_repo_get_contents_list_test.go +++ b/integrations/api_repo_get_contents_list_test.go @@ -74,6 +74,8 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) { repo1.CreateNewBranch(user2, repo1.DefaultBranch, newBranch) // Get the commit ID of the default branch gitRepo, _ := git.OpenRepository(repo1.RepoPath()) + defer gitRepo.Close() + commitID, _ := gitRepo.GetBranchCommitID(repo1.DefaultBranch) // Make a new tag in repo1 newTag := "test_tag" diff --git a/integrations/api_repo_get_contents_test.go b/integrations/api_repo_get_contents_test.go index f6a43bc5c63f2..77a827ec6125b 100644 --- a/integrations/api_repo_get_contents_test.go +++ b/integrations/api_repo_get_contents_test.go @@ -75,6 +75,8 @@ func testAPIGetContents(t *testing.T, u *url.URL) { repo1.CreateNewBranch(user2, repo1.DefaultBranch, newBranch) // Get the commit ID of the default branch gitRepo, _ := git.OpenRepository(repo1.RepoPath()) + defer gitRepo.Close() + commitID, _ := gitRepo.GetBranchCommitID(repo1.DefaultBranch) // Make a new tag in repo1 newTag := "test_tag" diff --git a/integrations/api_repo_git_tags_test.go b/integrations/api_repo_git_tags_test.go index ae519249e03b0..d6ff08990a37a 100644 --- a/integrations/api_repo_git_tags_test.go +++ b/integrations/api_repo_git_tags_test.go @@ -29,6 +29,8 @@ func TestAPIGitTags(t *testing.T) { git.NewCommand("config", "user.email", user.Email).RunInDir(repo.RepoPath()) gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() + commit, _ := gitRepo.GetBranchCommit("master") lTagName := "lightweightTag" gitRepo.CreateTag(lTagName, commit.ID.String()) diff --git a/integrations/repofiles_delete_test.go b/integrations/repofiles_delete_test.go index f4cb4510be447..10742bf0b9578 100644 --- a/integrations/repofiles_delete_test.go +++ b/integrations/repofiles_delete_test.go @@ -73,6 +73,7 @@ func testDeleteRepoFile(t *testing.T, u *url.URL) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() repo := ctx.Repo.Repository doer := ctx.User opts := getDeleteRepoFileOptions(repo) @@ -111,6 +112,8 @@ func testDeleteRepoFileWithoutBranchNames(t *testing.T, u *url.URL) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User opts := getDeleteRepoFileOptions(repo) @@ -139,6 +142,8 @@ func TestDeleteRepoFileErrors(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User diff --git a/integrations/repofiles_update_test.go b/integrations/repofiles_update_test.go index a4ce16d8479e9..5b88bb8198332 100644 --- a/integrations/repofiles_update_test.go +++ b/integrations/repofiles_update_test.go @@ -191,6 +191,8 @@ func TestCreateOrUpdateRepoFileForCreate(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User opts := getCreateRepoFileOptions(repo) @@ -201,6 +203,8 @@ func TestCreateOrUpdateRepoFileForCreate(t *testing.T) { // asserts assert.Nil(t, err) gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() + commitID, _ := gitRepo.GetBranchCommitID(opts.NewBranch) expectedFileResponse := getExpectedFileResponseForRepofilesCreate(commitID) assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content) @@ -220,6 +224,8 @@ func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User opts := getUpdateRepoFileOptions(repo) @@ -230,6 +236,8 @@ func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) { // asserts assert.Nil(t, err) gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() + commitID, _ := gitRepo.GetBranchCommitID(opts.NewBranch) expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commitID, opts.TreePath) assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content) @@ -249,6 +257,8 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User opts := getUpdateRepoFileOptions(repo) @@ -261,6 +271,8 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) { // asserts assert.Nil(t, err) gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() + commit, _ := gitRepo.GetBranchCommit(opts.NewBranch) expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.TreePath) // assert that the old file no longer exists in the last commit of the branch @@ -288,6 +300,8 @@ func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User opts := getUpdateRepoFileOptions(repo) @@ -300,6 +314,8 @@ func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) { // asserts assert.Nil(t, err) gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() + commitID, _ := gitRepo.GetBranchCommitID(repo.DefaultBranch) expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commitID, opts.TreePath) assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content) @@ -315,6 +331,8 @@ func TestCreateOrUpdateRepoFileErrors(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User diff --git a/models/action.go b/models/action.go index 7011f216c3e51..1855309b5ab5c 100644 --- a/models/action.go +++ b/models/action.go @@ -762,6 +762,7 @@ func CommitRepoAction(opts CommitRepoActionOptions) error { if err != nil { log.Error("GetBranchCommitID[%s]: %v", opts.RefFullName, err) } + gitRepo.Close() if err = PrepareWebhooks(repo, HookEventCreate, &api.CreatePayload{ Ref: refName, Sha: shaSum, @@ -797,6 +798,8 @@ func CommitRepoAction(opts CommitRepoActionOptions) error { if err != nil { log.Error("GetTagCommitID[%s]: %v", opts.RefFullName, err) } + gitRepo.Close() + if err = PrepareWebhooks(repo, HookEventCreate, &api.CreatePayload{ Ref: refName, Sha: shaSum, diff --git a/models/git_diff.go b/models/git_diff.go index 2f48f1b6fa7e8..c6f97b2d46c01 100644 --- a/models/git_diff.go +++ b/models/git_diff.go @@ -675,6 +675,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID if err != nil { return nil, err } + defer gitRepo.Close() commit, err := gitRepo.GetCommit(afterCommitID) if err != nil { @@ -747,6 +748,7 @@ func GetRawDiffForFile(repoPath, startCommit, endCommit string, diffType RawDiff if err != nil { return fmt.Errorf("OpenRepository: %v", err) } + defer repo.Close() commit, err := repo.GetCommit(endCommit) if err != nil { diff --git a/models/graph_test.go b/models/graph_test.go index 5c78e3877b576..4e104592a3a1b 100644 --- a/models/graph_test.go +++ b/models/graph_test.go @@ -17,6 +17,7 @@ func BenchmarkGetCommitGraph(b *testing.B) { if err != nil { b.Error("Could not open repository") } + defer currentRepo.Close() for i := 0; i < b.N; i++ { graph, err := GetCommitGraph(currentRepo) diff --git a/models/issue_comment.go b/models/issue_comment.go index 0a1d9852cf2fd..648a21a969dc5 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -893,6 +893,7 @@ func CreateCodeComment(doer *User, repo *Repository, issue *Issue, content, tree if err != nil { return nil, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() // FIXME validate treePath // Get latest commit referencing the commented line diff --git a/models/migrations/v39.go b/models/migrations/v39.go index 1312cb33134b5..992e06bcaae12 100644 --- a/models/migrations/v39.go +++ b/models/migrations/v39.go @@ -47,6 +47,7 @@ func releaseAddColumnIsTagAndSyncTags(x *xorm.Engine) error { if err = models.SyncReleasesWithTags(repo, gitRepo); err != nil { log.Warn("SyncReleasesWithTags: %v", err) } + gitRepo.Close() } if len(repos) < pageSize { break diff --git a/models/migrations/v82.go b/models/migrations/v82.go index eb73f1834371f..83da703c8a0f2 100644 --- a/models/migrations/v82.go +++ b/models/migrations/v82.go @@ -91,6 +91,7 @@ func fixReleaseSha1OnReleaseTable(x *xorm.Engine) error { if err != nil { return err } + defer gitRepo.Close() gitRepoCache[release.RepoID] = gitRepo } diff --git a/models/pull.go b/models/pull.go index 13518dd1c391d..7a1c4ee6b2847 100644 --- a/models/pull.go +++ b/models/pull.go @@ -338,6 +338,7 @@ func (pr *PullRequest) GetLastCommitStatus() (status *CommitStatus, err error) { if err != nil { return nil, err } + defer headGitRepo.Close() lastCommitID, err := headGitRepo.GetBranchCommitID(pr.HeadBranch) if err != nil { @@ -527,6 +528,7 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) { if err != nil { return nil, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() commit, err := gitRepo.GetCommit(mergeCommit[:40]) if err != nil { @@ -920,6 +922,7 @@ func (pr *PullRequest) UpdatePatch() (err error) { if err != nil { return fmt.Errorf("OpenRepository: %v", err) } + defer headGitRepo.Close() // Add a temporary remote. tmpRemote := com.ToStr(time.Now().UnixNano()) @@ -961,6 +964,7 @@ func (pr *PullRequest) PushToBaseRepo() (err error) { if err != nil { return fmt.Errorf("OpenRepository: %v", err) } + defer headGitRepo.Close() tmpRemoteName := fmt.Sprintf("tmp-pull-%d", pr.ID) if err = headGitRepo.AddRemote(tmpRemoteName, pr.BaseRepo.RepoPath(), false); err != nil { @@ -1150,6 +1154,7 @@ func checkForInvalidation(requests PullRequestList, repoID int64, doer *User, br if err != nil { log.Error("PullRequestList.InvalidateCodeComments: %v", err) } + gitRepo.Close() }() return nil } diff --git a/models/release_test.go b/models/release_test.go index 83c3fe2f77c41..0473bbdfa1e76 100644 --- a/models/release_test.go +++ b/models/release_test.go @@ -21,6 +21,7 @@ func TestRelease_Create(t *testing.T) { gitRepo, err := git.OpenRepository(repoPath) assert.NoError(t, err) + defer gitRepo.Close() assert.NoError(t, CreateRelease(gitRepo, &Release{ RepoID: repo.ID, @@ -115,6 +116,7 @@ func TestRelease_MirrorDelete(t *testing.T) { gitRepo, err := git.OpenRepository(repoPath) assert.NoError(t, err) + defer gitRepo.Close() findOptions := FindReleasesOptions{IncludeDrafts: true, IncludeTags: true} initCount, err := GetReleaseCountByRepoID(mirror.ID, findOptions) diff --git a/models/repo.go b/models/repo.go index 5d17a043b86a9..4fb1a50b5abbc 100644 --- a/models/repo.go +++ b/models/repo.go @@ -950,6 +950,7 @@ func MigrateRepository(doer, u *User, opts MigrateRepoOptions) (*Repository, err if err != nil { return repo, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() repo.IsEmpty, err = gitRepo.IsEmpty() if err != nil { diff --git a/models/repo_activity.go b/models/repo_activity.go index 04612ae1efca0..c95af85495b7f 100644 --- a/models/repo_activity.go +++ b/models/repo_activity.go @@ -64,6 +64,8 @@ func GetActivityStats(repo *Repository, timeFrom time.Time, releases, issues, pr if err != nil { return nil, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() + code, err := gitRepo.GetCodeActivityStats(timeFrom, repo.DefaultBranch) if err != nil { return nil, fmt.Errorf("FillFromGit: %v", err) @@ -79,6 +81,8 @@ func GetActivityStatsTopAuthors(repo *Repository, timeFrom time.Time, count int) if err != nil { return nil, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() + code, err := gitRepo.GetCodeActivityStats(timeFrom, "") if err != nil { return nil, fmt.Errorf("FillFromGit: %v", err) diff --git a/models/repo_branch.go b/models/repo_branch.go index dee6ef3d7e267..c51323183688d 100644 --- a/models/repo_branch.go +++ b/models/repo_branch.go @@ -23,6 +23,7 @@ func (repo *Repository) GetBranch(branch string) (*git.Branch, error) { if err != nil { return nil, err } + defer gitRepo.Close() return gitRepo.GetBranch(branch) } @@ -38,6 +39,7 @@ func (repo *Repository) CheckBranchName(name string) error { if err != nil { return err } + defer gitRepo.Close() branches, err := repo.GetBranches() if err != nil { @@ -94,6 +96,7 @@ func (repo *Repository) CreateNewBranch(doer *User, oldBranchName, branchName st log.Error("Unable to open temporary repository: %s (%v)", basePath, err) return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err) } + defer gitRepo.Close() if err = gitRepo.CreateBranch(branchName, oldBranchName); err != nil { 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 log.Error("Unable to open temporary repository: %s (%v)", basePath, err) return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err) } + defer gitRepo.Close() if err = gitRepo.CreateBranch(branchName, commit); err != nil { log.Error("Unable to create branch: %s from %s. (%v)", branchName, commit, err) diff --git a/models/repo_mirror.go b/models/repo_mirror.go index 982672ea2b83e..191f758af3a0e 100644 --- a/models/repo_mirror.go +++ b/models/repo_mirror.go @@ -242,6 +242,7 @@ func (m *Mirror) runSync() ([]*mirrorSyncResult, bool) { if err = SyncReleasesWithTags(m.Repo, gitRepo); err != nil { log.Error("Failed to synchronize tags to releases for repository: %v", err) } + gitRepo.Close() if err := m.Repo.UpdateSize(); err != nil { log.Error("Failed to update size for mirror repository: %v", err) @@ -426,6 +427,8 @@ func SyncMirrors() { } } + gitRepo.Close() + // Get latest commit date and update to current repository updated time commitDate, err := git.GetLatestCommitTime(m.Repo.RepoPath()) if err != nil { diff --git a/models/repo_tag.go b/models/repo_tag.go deleted file mode 100644 index 3864b7a12a7c9..0000000000000 --- a/models/repo_tag.go +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2019 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package models - -import ( - "code.gitea.io/gitea/modules/git" -) - -// GetTagsByPath returns repo tags by its path -func GetTagsByPath(path string) ([]*git.Tag, error) { - gitRepo, err := git.OpenRepository(path) - if err != nil { - return nil, err - } - - return gitRepo.GetTagInfos() -} - -// GetTags return repo's tags -func (repo *Repository) GetTags() ([]*git.Tag, error) { - return GetTagsByPath(repo.RepoPath()) -} diff --git a/models/update.go b/models/update.go index 411f7d5be14a8..c08addacebe21 100644 --- a/models/update.go +++ b/models/update.go @@ -211,6 +211,7 @@ func pushUpdate(opts PushUpdateOptions) (repo *Repository, err error) { if err != nil { return nil, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() if err = repo.UpdateSize(); err != nil { log.Error("Failed to update size for repository: %v", err) diff --git a/models/wiki.go b/models/wiki.go index d47f58f5a3afa..35315da8890a0 100644 --- a/models/wiki.go +++ b/models/wiki.go @@ -140,6 +140,7 @@ func (repo *Repository) updateWikiPage(doer *User, oldWikiName, newWikiName, con log.Error("Unable to open temporary repository: %s (%v)", basePath, err) return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err) } + defer gitRepo.Close() if hasMasterBranch { if err := gitRepo.ReadTreeToIndex("HEAD"); err != nil { @@ -276,6 +277,7 @@ func (repo *Repository) DeleteWikiPage(doer *User, wikiName string) (err error) log.Error("Unable to open temporary repository: %s (%v)", basePath, err) return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err) } + defer gitRepo.Close() if err := gitRepo.ReadTreeToIndex("HEAD"); err != nil { log.Error("Unable to read HEAD tree to index in: %s %v", basePath, err) diff --git a/models/wiki_test.go b/models/wiki_test.go index 991a3d95b9036..37c0a86635ea7 100644 --- a/models/wiki_test.go +++ b/models/wiki_test.go @@ -161,6 +161,7 @@ func TestRepository_AddWikiPage(t *testing.T) { // Now need to show that the page has been added: gitRepo, err := git.OpenRepository(repo.WikiPath()) assert.NoError(t, err) + defer gitRepo.Close() masterTree, err := gitRepo.GetTree("master") assert.NoError(t, err) wikiPath := WikiNameToFilename(wikiName) @@ -214,6 +215,7 @@ func TestRepository_EditWikiPage(t *testing.T) { _, err := masterTree.GetTreeEntryByPath("Home.md") assert.Error(t, err) } + gitRepo.Close() } } @@ -226,6 +228,7 @@ func TestRepository_DeleteWikiPage(t *testing.T) { // Now need to show that the page has been added: gitRepo, err := git.OpenRepository(repo.WikiPath()) assert.NoError(t, err) + defer gitRepo.Close() masterTree, err := gitRepo.GetTree("master") assert.NoError(t, err) wikiPath := WikiNameToFilename("Home") diff --git a/modules/context/api.go b/modules/context/api.go index 0b2f81fd64684..0ff213440d204 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -187,7 +187,16 @@ func ReferencesGitRepo(allowEmpty bool) macaron.Handler { return } ctx.Repo.GitRepo = gitRepo + // We opened it, we should close it + defer func() { + // If it's been set to nil then assume someone else has closed it. + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + } + }() } + + ctx.Next() } } diff --git a/modules/context/repo.go b/modules/context/repo.go index 1131cf4f47310..1339a3253619e 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -364,6 +364,13 @@ func RepoAssignment() macaron.Handler { return } ctx.Repo.GitRepo = gitRepo + // We opened it, we should close it + defer func() { + // If it's been set to nil then assume someone else has closed it. + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + } + }() ctx.Repo.RepoLink = repo.Link() ctx.Data["RepoLink"] = ctx.Repo.RepoLink ctx.Data["RepoRelPath"] = ctx.Repo.Owner.Name + "/" + ctx.Repo.Repository.Name @@ -426,6 +433,7 @@ func RepoAssignment() macaron.Handler { // repo is empty and display enable if ctx.Repo.Repository.IsEmpty { ctx.Data["BranchName"] = ctx.Repo.Repository.DefaultBranch + ctx.Next() return } @@ -476,6 +484,7 @@ func RepoAssignment() macaron.Handler { ctx.Data["GoDocDirectory"] = prefix + "{/dir}" ctx.Data["GoDocFile"] = prefix + "{/dir}/{file}#L{line}" } + ctx.Next() } } @@ -581,6 +590,13 @@ func RepoRefByType(refType RepoRefType) macaron.Handler { ctx.ServerError("RepoRef Invalid repo "+repoPath, err) return } + // We opened it, we should close it + defer func() { + // If it's been set to nil then assume someone else has closed it. + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + } + }() } // Get default branch. @@ -669,6 +685,8 @@ func RepoRefByType(refType RepoRefType) macaron.Handler { return } ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount + + ctx.Next() } } diff --git a/modules/git/blame.go b/modules/git/blame.go index 548236b65702a..4f4343fe96426 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -87,10 +87,11 @@ func (r *BlameReader) Close() error { // CreateBlameReader creates reader for given repository, commit and file func CreateBlameReader(repoPath, commitID, file string) (*BlameReader, error) { - _, err := OpenRepository(repoPath) + gitRepo, err := OpenRepository(repoPath) if err != nil { return nil, err } + gitRepo.Close() return createBlameReader(repoPath, GitExecutable, "blame", commitID, "--porcelain", "--", file) } diff --git a/modules/git/blob_test.go b/modules/git/blob_test.go index 66c046ecc8d08..9043de5955f49 100644 --- a/modules/git/blob_test.go +++ b/modules/git/blob_test.go @@ -37,6 +37,8 @@ THE SOFTWARE. ` repo, err := OpenRepository("../../.git") assert.NoError(t, err) + defer repo.Close() + testBlob, err := repo.GetBlob("a8d4b49dd073a4a38a7e58385eeff7cc52568697") assert.NoError(t, err) @@ -55,6 +57,8 @@ func Benchmark_Blob_Data(b *testing.B) { if err != nil { b.Fatal(err) } + defer repo.Close() + testBlob, err := repo.GetBlob("a8d4b49dd073a4a38a7e58385eeff7cc52568697") if err != nil { b.Fatal(err) diff --git a/modules/git/commit_info_test.go b/modules/git/commit_info_test.go index 71637d188aaa1..ac7bc43c4e214 100644 --- a/modules/git/commit_info_test.go +++ b/modules/git/commit_info_test.go @@ -77,6 +77,8 @@ func TestEntries_GetCommitsInfo(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() + testGetCommitsInfo(t, bareRepo1) clonedPath, err := cloneRepo(bareRepo1Path, testReposDir, "repo1_TestEntries_GetCommitsInfo") @@ -84,6 +86,8 @@ func TestEntries_GetCommitsInfo(t *testing.T) { defer os.RemoveAll(clonedPath) clonedRepo1, err := OpenRepository(clonedPath) assert.NoError(t, err) + defer clonedRepo1.Close() + testGetCommitsInfo(t, clonedRepo1) } @@ -101,13 +105,16 @@ func BenchmarkEntries_GetCommitsInfo(b *testing.B) { for _, benchmark := range benchmarks { var commit *Commit var entries Entries + var repo *Repository if repoPath, err := cloneRepo(benchmark.url, benchmarkReposDir, benchmark.name); err != nil { b.Fatal(err) - } else if repo, err := OpenRepository(repoPath); err != nil { + } else if repo, err = OpenRepository(repoPath); err != nil { b.Fatal(err) } else if commit, err = repo.GetBranchCommit("master"); err != nil { + repo.Close() b.Fatal(err) } else if entries, err = commit.Tree.ListEntries(); err != nil { + repo.Close() b.Fatal(err) } entries.Sort() @@ -120,5 +127,6 @@ func BenchmarkEntries_GetCommitsInfo(b *testing.B) { } } }) + repo.Close() } } diff --git a/modules/git/notes_test.go b/modules/git/notes_test.go index bf010b9a712aa..b7939e691355a 100644 --- a/modules/git/notes_test.go +++ b/modules/git/notes_test.go @@ -15,6 +15,7 @@ func TestGetNotes(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() note := Note{} err = GetNote(bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653", ¬e) @@ -27,6 +28,7 @@ func TestGetNestedNotes(t *testing.T) { repoPath := filepath.Join(testReposDir, "repo3_notes") repo, err := OpenRepository(repoPath) assert.NoError(t, err) + defer repo.Close() note := Note{} err = GetNote(repo, "3e668dbfac39cbc80a9ff9c61eb565d944453ba4", ¬e) diff --git a/modules/git/repo.go b/modules/git/repo.go index 28e54a1bbcf59..7adc622fca8ad 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -17,6 +17,7 @@ import ( "strings" "time" + gitealog "code.gitea.io/gitea/modules/log" "github.com/Unknwon/com" "gopkg.in/src-d/go-billy.v4/osfs" gogit "gopkg.in/src-d/go-git.v4" @@ -107,6 +108,21 @@ func OpenRepository(repoPath string) (*Repository, error) { }, nil } +// Close this repository, in particular close the underlying gogitStorage if this is not nil +func (repo *Repository) Close() { + if repo == nil || repo.gogitStorage == nil { + return + } + if err := repo.gogitStorage.Close(); err != nil { + gitealog.Error("Error closing storage: %v", err) + } +} + +// GoGitRepo gets the go-git repo representation +func (repo *Repository) GoGitRepo() *gogit.Repository { + return repo.gogitRepo +} + // IsEmpty Check if repository is empty. func (repo *Repository) IsEmpty() (bool, error) { var errbuf strings.Builder diff --git a/modules/git/repo_blob_test.go b/modules/git/repo_blob_test.go index 128a227829c1c..52a124db2a750 100644 --- a/modules/git/repo_blob_test.go +++ b/modules/git/repo_blob_test.go @@ -17,6 +17,7 @@ func TestRepository_GetBlob_Found(t *testing.T) { repoPath := filepath.Join(testReposDir, "repo1_bare") r, err := OpenRepository(repoPath) assert.NoError(t, err) + defer r.Close() testCases := []struct { OID string @@ -44,6 +45,7 @@ func TestRepository_GetBlob_NotExist(t *testing.T) { repoPath := filepath.Join(testReposDir, "repo1_bare") r, err := OpenRepository(repoPath) assert.NoError(t, err) + defer r.Close() testCase := "0000000000000000000000000000000000000000" testError := ErrNotExist{testCase, ""} @@ -57,6 +59,7 @@ func TestRepository_GetBlob_NoId(t *testing.T) { repoPath := filepath.Join(testReposDir, "repo1_bare") r, err := OpenRepository(repoPath) assert.NoError(t, err) + defer r.Close() testCase := "" testError := fmt.Errorf("Length must be 40: %s", testCase) diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index a2bf9ac973e49..e79bab76a6f30 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -108,6 +108,7 @@ func GetBranchesByPath(path string) ([]*Branch, error) { if err != nil { return nil, err } + defer gitRepo.Close() brs, err := gitRepo.GetBranches() if err != nil { diff --git a/modules/git/repo_branch_test.go b/modules/git/repo_branch_test.go index 08736d702e211..33d31aef686de 100644 --- a/modules/git/repo_branch_test.go +++ b/modules/git/repo_branch_test.go @@ -15,6 +15,7 @@ func TestRepository_GetBranches(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() branches, err := bareRepo1.GetBranches() @@ -29,6 +30,7 @@ func BenchmarkRepository_GetBranches(b *testing.B) { if err != nil { b.Fatal(err) } + defer bareRepo1.Close() for i := 0; i < b.N; i++ { _, err := bareRepo1.GetBranches() diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go index c59e143cc3169..82aa0a0f3c725 100644 --- a/modules/git/repo_commit_test.go +++ b/modules/git/repo_commit_test.go @@ -15,6 +15,7 @@ func TestRepository_GetCommitBranches(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() // these test case are specific to the repo1_bare test repo testCases := []struct { @@ -40,6 +41,9 @@ func TestRepository_GetCommitBranches(t *testing.T) { func TestGetTagCommitWithSignature(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) + assert.NoError(t, err) + defer bareRepo1.Close() + commit, err := bareRepo1.GetCommit("3ad28a9149a2864384548f3d17ed7f38014c9e8a") assert.NoError(t, err) @@ -52,6 +56,9 @@ func TestGetTagCommitWithSignature(t *testing.T) { func TestGetCommitWithBadCommitID(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) + assert.NoError(t, err) + defer bareRepo1.Close() + commit, err := bareRepo1.GetCommit("bad_branch") assert.Nil(t, commit) assert.Error(t, err) diff --git a/modules/git/repo_compare_test.go b/modules/git/repo_compare_test.go index e19478877341f..def67fa87b7f3 100644 --- a/modules/git/repo_compare_test.go +++ b/modules/git/repo_compare_test.go @@ -20,6 +20,7 @@ func TestGetFormatPatch(t *testing.T) { defer os.RemoveAll(clonedPath) repo, err := OpenRepository(clonedPath) assert.NoError(t, err) + defer repo.Close() rd, err := repo.GetFormatPatch("8d92fc95^", "8d92fc95") assert.NoError(t, err) patchb, err := ioutil.ReadAll(rd) diff --git a/modules/git/repo_ref_test.go b/modules/git/repo_ref_test.go index d32b34994c391..303c496c1ddfa 100644 --- a/modules/git/repo_ref_test.go +++ b/modules/git/repo_ref_test.go @@ -15,6 +15,7 @@ func TestRepository_GetRefs(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() refs, err := bareRepo1.GetRefs() @@ -38,6 +39,7 @@ func TestRepository_GetRefsFiltered(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() refs, err := bareRepo1.GetRefsFiltered(TagPrefix) diff --git a/modules/git/repo_stats_test.go b/modules/git/repo_stats_test.go index 1822af0be6c39..31b0c6ee2b0fa 100644 --- a/modules/git/repo_stats_test.go +++ b/modules/git/repo_stats_test.go @@ -16,6 +16,7 @@ func TestRepository_GetCodeActivityStats(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() timeFrom, err := time.Parse(time.RFC3339, "2016-01-01T00:00:00+00:00") diff --git a/modules/git/repo_tag_test.go b/modules/git/repo_tag_test.go index ab9742afc5963..90f2b37358d77 100644 --- a/modules/git/repo_tag_test.go +++ b/modules/git/repo_tag_test.go @@ -16,6 +16,7 @@ func TestRepository_GetTags(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() tags, err := bareRepo1.GetTagInfos() assert.NoError(t, err) @@ -34,6 +35,7 @@ func TestRepository_GetTag(t *testing.T) { bareRepo1, err := OpenRepository(clonedPath) assert.NoError(t, err) + defer bareRepo1.Close() lTagCommitID := "6fbd69e9823458e6c4a2fc5c0f6bc022b2f2acd1" lTagName := "lightweightTag" @@ -83,6 +85,7 @@ func TestRepository_GetAnnotatedTag(t *testing.T) { bareRepo1, err := OpenRepository(clonedPath) assert.NoError(t, err) + defer bareRepo1.Close() lTagCommitID := "6fbd69e9823458e6c4a2fc5c0f6bc022b2f2acd1" lTagName := "lightweightTag" diff --git a/modules/git/repo_test.go b/modules/git/repo_test.go index 15f5e3781c050..0b6986764c05d 100644 --- a/modules/git/repo_test.go +++ b/modules/git/repo_test.go @@ -30,6 +30,7 @@ func TestRepoIsEmpty(t *testing.T) { emptyRepo2Path := filepath.Join(testReposDir, "repo2_empty") repo, err := OpenRepository(emptyRepo2Path) assert.NoError(t, err) + defer repo.Close() isEmpty, err := repo.IsEmpty() assert.NoError(t, err) assert.True(t, isEmpty) diff --git a/modules/git/tree_entry_test.go b/modules/git/tree_entry_test.go index c65a691ecf203..e8729003703e1 100644 --- a/modules/git/tree_entry_test.go +++ b/modules/git/tree_entry_test.go @@ -56,6 +56,7 @@ func TestEntriesCustomSort(t *testing.T) { func TestFollowLink(t *testing.T) { r, err := OpenRepository("tests/repos/repo1_bare") assert.NoError(t, err) + defer r.Close() commit, err := r.GetCommit("37991dec2c8e592043f47155ce4808d4580f9123") assert.NoError(t, err) diff --git a/modules/migrations/base/uploader.go b/modules/migrations/base/uploader.go index 8c1d649229fc6..32fffea339971 100644 --- a/modules/migrations/base/uploader.go +++ b/modules/migrations/base/uploader.go @@ -16,4 +16,5 @@ type Uploader interface { CreateComments(comments ...*Comment) error CreatePullRequests(prs ...*PullRequest) error Rollback() error + Close() } diff --git a/modules/migrations/gitea.go b/modules/migrations/gitea.go index 374e4b34d6fba..389e466910388 100644 --- a/modules/migrations/gitea.go +++ b/modules/migrations/gitea.go @@ -107,6 +107,13 @@ func (g *GiteaLocalUploader) CreateRepo(repo *base.Repository, opts base.Migrate return err } +// Close closes this uploader +func (g *GiteaLocalUploader) Close() { + if g.gitRepo != nil { + g.gitRepo.Close() + } +} + // CreateMilestones creates milestones func (g *GiteaLocalUploader) CreateMilestones(milestones ...*base.Milestone) error { var mss = make([]*models.Milestone, 0, len(milestones)) diff --git a/modules/migrations/migrate.go b/modules/migrations/migrate.go index 7fc7911f2189d..e39a231e03ced 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -83,6 +83,7 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts if err := uploader.CreateRepo(repo, opts); err != nil { return err } + defer uploader.Close() if opts.Milestones { log.Trace("migrating milestones") diff --git a/modules/repofiles/blob.go b/modules/repofiles/blob.go index e9d85a0dcf25d..60a05e280e864 100644 --- a/modules/repofiles/blob.go +++ b/modules/repofiles/blob.go @@ -17,6 +17,7 @@ func GetBlobBySHA(repo *models.Repository, sha string) (*api.GitBlobResponse, er if err != nil { return nil, err } + defer gitRepo.Close() gitBlob, err := gitRepo.GetBlob(sha) if err != nil { return nil, err diff --git a/modules/repofiles/blob_test.go b/modules/repofiles/blob_test.go index 1dc183a8afb30..ddc23aeac3518 100644 --- a/modules/repofiles/blob_test.go +++ b/modules/repofiles/blob_test.go @@ -21,6 +21,8 @@ func TestGetBlobBySHA(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d" ctx.SetParams(":id", "1") ctx.SetParams(":sha", sha) diff --git a/modules/repofiles/commit_status.go b/modules/repofiles/commit_status.go index f3dfbf209f7dd..3d93c58d8582c 100644 --- a/modules/repofiles/commit_status.go +++ b/modules/repofiles/commit_status.go @@ -23,8 +23,10 @@ func CreateCommitStatus(repo *models.Repository, creator *models.User, sha strin return fmt.Errorf("OpenRepository[%s]: %v", repoPath, err) } if _, err := gitRepo.GetCommit(sha); err != nil { + gitRepo.Close() return fmt.Errorf("GetCommit[%s]: %v", sha, err) } + gitRepo.Close() if err := models.NewCommitStatus(models.NewCommitStatusOptions{ Repo: repo, diff --git a/modules/repofiles/content.go b/modules/repofiles/content.go index d7d43ef9d1661..aed98c33a8c4a 100644 --- a/modules/repofiles/content.go +++ b/modules/repofiles/content.go @@ -59,6 +59,7 @@ func GetContentsOrList(repo *models.Repository, treePath, ref string) (interface if err != nil { return nil, err } + defer gitRepo.Close() // Get the commit object for the ref commit, err := gitRepo.GetCommit(ref) @@ -117,6 +118,7 @@ func GetContents(repo *models.Repository, treePath, ref string, forList bool) (* if err != nil { return nil, err } + defer gitRepo.Close() // Get the commit object for the ref commit, err := gitRepo.GetCommit(ref) diff --git a/modules/repofiles/content_test.go b/modules/repofiles/content_test.go index cd98c54ea69f6..d024cfd54982f 100644 --- a/modules/repofiles/content_test.go +++ b/modules/repofiles/content_test.go @@ -56,6 +56,8 @@ func TestGetContents(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + treePath := "README.md" ref := ctx.Repo.Repository.DefaultBranch @@ -82,6 +84,8 @@ func TestGetContentsOrListForDir(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + treePath := "" // root dir ref := ctx.Repo.Repository.DefaultBranch @@ -115,6 +119,8 @@ func TestGetContentsOrListForFile(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + treePath := "README.md" ref := ctx.Repo.Repository.DefaultBranch @@ -141,6 +147,8 @@ func TestGetContentsErrors(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository treePath := "README.md" ref := repo.DefaultBranch @@ -170,6 +178,8 @@ func TestGetContentsOrListErrors(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository treePath := "README.md" ref := repo.DefaultBranch @@ -198,6 +208,8 @@ func TestGetContentsOrListOfEmptyRepos(t *testing.T) { test.LoadRepo(t, ctx, 15) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository t.Run("empty repo", func(t *testing.T) { diff --git a/modules/repofiles/diff_test.go b/modules/repofiles/diff_test.go index bc7d4ebad6662..6cf4981536b48 100644 --- a/modules/repofiles/diff_test.go +++ b/modules/repofiles/diff_test.go @@ -21,6 +21,8 @@ func TestGetDiffPreview(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + branch := ctx.Repo.Repository.DefaultBranch treePath := "README.md" content := "# repo1\n\nDescription for repo1\nthis is a new line" @@ -118,6 +120,8 @@ func TestGetDiffPreviewErrors(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + branch := ctx.Repo.Repository.DefaultBranch treePath := "README.md" content := "# repo1\n\nDescription for repo1\nthis is a new line" diff --git a/modules/repofiles/file_test.go b/modules/repofiles/file_test.go index 00feb93fffa34..830f2f7b7e465 100644 --- a/modules/repofiles/file_test.go +++ b/modules/repofiles/file_test.go @@ -88,10 +88,13 @@ func TestGetFileResponseFromCommit(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository branch := repo.DefaultBranch treePath := "README.md" gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() commit, _ := gitRepo.GetBranchCommit(branch) expectedFileResponse := getExpectedFileResponse() diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index 28f3836d0e119..3b4ed330ec148 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -42,6 +42,7 @@ func NewTemporaryUploadRepository(repo *models.Repository) (*TemporaryUploadRepo // Close the repository cleaning up all files func (t *TemporaryUploadRepository) Close() { + defer t.gitRepo.Close() if err := models.RemoveTemporaryPath(t.basePath); err != nil { log.Error("Failed to remove temporary path %s: %v", t.basePath, err) } diff --git a/modules/repofiles/tree.go b/modules/repofiles/tree.go index 318a5d152d839..cf0534563fc37 100644 --- a/modules/repofiles/tree.go +++ b/modules/repofiles/tree.go @@ -19,6 +19,7 @@ func GetTreeBySHA(repo *models.Repository, sha string, page, perPage int, recurs if err != nil { return nil, err } + defer gitRepo.Close() gitTree, err := gitRepo.GetTree(sha) if err != nil || gitTree == nil { return nil, models.ErrSHANotFound{ diff --git a/modules/repofiles/tree_test.go b/modules/repofiles/tree_test.go index ecff8b90717e8..e1bb812ec156b 100644 --- a/modules/repofiles/tree_test.go +++ b/modules/repofiles/tree_test.go @@ -21,6 +21,8 @@ func TestGetTreeBySHA(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + sha := ctx.Repo.Repository.DefaultBranch page := 1 perPage := 10 diff --git a/modules/test/context_tests.go b/modules/test/context_tests.go index d5a800d360985..e8bddc3d1026b 100644 --- a/modules/test/context_tests.go +++ b/modules/test/context_tests.go @@ -55,6 +55,7 @@ func LoadRepo(t *testing.T, ctx *context.Context, repoID int64) { func LoadRepoCommit(t *testing.T, ctx *context.Context) { gitRepo, err := git.OpenRepository(ctx.Repo.Repository.RepoPath()) assert.NoError(t, err) + defer gitRepo.Close() branch, err := gitRepo.GetHEADBranch() assert.NoError(t, err) ctx.Repo.Commit, err = gitRepo.GetBranchCommit(branch.Name) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 363379381ad25..2d944b2e366e0 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -637,7 +637,7 @@ func RegisterRoutes(m *macaron.Macaron) { }, reqRepoReader(models.UnitTypeCode)) m.Group("/tags", func() { m.Get("", repo.ListTags) - }, reqRepoReader(models.UnitTypeCode)) + }, reqRepoReader(models.UnitTypeCode), context.ReferencesGitRepo(true)) m.Group("/keys", func() { m.Combo("").Get(repo.ListDeployKeys). Post(bind(api.CreateKeyOption{}), repo.CreateDeployKey) diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index 795ac1f22feb5..6144e1eeda269 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -49,6 +49,7 @@ func GetSingleCommit(ctx *context.APIContext) { ctx.ServerError("OpenRepository", err) return } + defer gitRepo.Close() commit, err := gitRepo.GetCommit(ctx.Params(":sha")) if err != nil { ctx.NotFoundOrServerError("GetCommit", git.IsErrNotExist, err) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index ae20e1e96be9f..f23e7bca9ea1f 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -95,6 +95,7 @@ func GetArchive(ctx *context.APIContext) { return } ctx.Repo.GitRepo = gitRepo + defer gitRepo.Close() repo.Download(ctx.Context) } diff --git a/routers/api/v1/repo/git_ref.go b/routers/api/v1/repo/git_ref.go index d7acc139f0d57..c2bcbb3603864 100644 --- a/routers/api/v1/repo/git_ref.go +++ b/routers/api/v1/repo/git_ref.go @@ -76,6 +76,8 @@ func getGitRefs(ctx *context.APIContext, filter string) ([]*git.Reference, strin if err != nil { return nil, "OpenRepository", err } + defer gitRepo.Close() + if len(filter) > 0 { filter = "refs/" + filter } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 4ad3479fd0901..bf8c93b8f9261 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -193,6 +193,7 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption if ctx.Written() { return } + defer headGitRepo.Close() // Check if another PR exists with the same targets existingPr, err := models.GetUnmergedPullRequest(headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch) @@ -683,6 +684,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) // user should have permission to read baseRepo's codes and pulls, NOT headRepo's permBase, err := models.GetUserRepoPermission(baseRepo, ctx.User) if err != nil { + headGitRepo.Close() ctx.ServerError("GetUserRepoPermission", err) return nil, nil, nil, nil, "", "" } @@ -693,6 +695,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) baseRepo, permBase) } + headGitRepo.Close() ctx.NotFound("Can't read pulls or can't read UnitTypeCode") return nil, nil, nil, nil, "", "" } @@ -700,6 +703,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) // user should have permission to read headrepo's codes permHead, err := models.GetUserRepoPermission(headRepo, ctx.User) if err != nil { + headGitRepo.Close() ctx.ServerError("GetUserRepoPermission", err) return nil, nil, nil, nil, "", "" } @@ -710,18 +714,21 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) headRepo, permHead) } + headGitRepo.Close() ctx.NotFound("Can't read headRepo UnitTypeCode") return nil, nil, nil, nil, "", "" } // Check if head branch is valid. if !headGitRepo.IsBranchExist(headBranch) { + headGitRepo.Close() ctx.NotFound() return nil, nil, nil, nil, "", "" } compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch) if err != nil { + headGitRepo.Close() ctx.Error(500, "GetCompareInfo", err) return nil, nil, nil, nil, "", "" } diff --git a/routers/api/v1/repo/tag.go b/routers/api/v1/repo/tag.go index ecf580e1b08c4..160793a2c0985 100644 --- a/routers/api/v1/repo/tag.go +++ b/routers/api/v1/repo/tag.go @@ -33,7 +33,7 @@ func ListTags(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/TagList" - tags, err := ctx.Repo.Repository.GetTags() + tags, err := ctx.Repo.GitRepo.GetTagInfos() if err != nil { ctx.Error(500, "GetTags", err) return diff --git a/routers/repo/compare.go b/routers/repo/compare.go index 193255ca6cad1..f65cbe81810db 100644 --- a/routers/repo/compare.go +++ b/routers/repo/compare.go @@ -116,6 +116,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * // user should have permission to read baseRepo's codes and pulls, NOT headRepo's permBase, err := models.GetUserRepoPermission(baseRepo, ctx.User) if err != nil { + headGitRepo.Close() ctx.ServerError("GetUserRepoPermission", err) return nil, nil, nil, nil, "", "" } @@ -126,6 +127,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * baseRepo, permBase) } + headGitRepo.Close() ctx.NotFound("ParseCompareInfo", nil) return nil, nil, nil, nil, "", "" } @@ -133,6 +135,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * // user should have permission to read headrepo's codes permHead, err := models.GetUserRepoPermission(headRepo, ctx.User) if err != nil { + headGitRepo.Close() ctx.ServerError("GetUserRepoPermission", err) return nil, nil, nil, nil, "", "" } @@ -143,6 +146,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * headRepo, permHead) } + headGitRepo.Close() ctx.NotFound("ParseCompareInfo", nil) return nil, nil, nil, nil, "", "" } @@ -158,6 +162,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * ctx.Data["HeadBranch"] = headBranch headIsCommit = true } else { + headGitRepo.Close() ctx.NotFound("IsRefExist", nil) return nil, nil, nil, nil, "", "" } @@ -178,12 +183,14 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * baseRepo, permBase) } + headGitRepo.Close() ctx.NotFound("ParseCompareInfo", nil) return nil, nil, nil, nil, "", "" } compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch) if err != nil { + headGitRepo.Close() ctx.ServerError("GetCompareInfo", err) return nil, nil, nil, nil, "", "" } @@ -285,6 +292,7 @@ func CompareDiff(ctx *context.Context) { if ctx.Written() { return } + defer headGitRepo.Close() nothingToCompare := PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch) if ctx.Written() { diff --git a/routers/repo/editor_test.go b/routers/repo/editor_test.go index 7a68ecc9b78f8..51fb558db4ffa 100644 --- a/routers/repo/editor_test.go +++ b/routers/repo/editor_test.go @@ -48,6 +48,8 @@ func TestGetUniquePatchBranchName(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + expectedBranchName := "user2-patch-1" branchName := GetUniquePatchBranchName(ctx) assert.Equal(t, expectedBranchName, branchName) @@ -61,9 +63,12 @@ func TestGetClosestParentWithFiles(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository branch := repo.DefaultBranch gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() commit, _ := gitRepo.GetBranchCommit(branch) expectedTreePath := "" diff --git a/routers/repo/pull.go b/routers/repo/pull.go index cb4fa9547e70d..7b284306f76ff 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -330,6 +330,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare ctx.ServerError("OpenRepository", err) return nil } + defer headGitRepo.Close() headBranchExist = headGitRepo.IsBranchExist(pull.HeadBranch) @@ -500,6 +501,7 @@ func ViewPullFiles(ctx *context.Context) { ctx.ServerError("OpenRepository", err) return } + defer headGitRepo.Close() headCommitID, err := headGitRepo.GetBranchCommitID(pull.HeadBranch) if err != nil { @@ -670,6 +672,7 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm) if ctx.Written() { return } + defer headGitRepo.Close() labelIDs, assigneeIDs, milestoneID := ValidateRepoMetas(ctx, form, true) if ctx.Written() { @@ -844,12 +847,14 @@ func CleanUpPullRequest(ctx *context.Context) { ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err) return } + defer gitRepo.Close() gitBaseRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) if err != nil { ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.BaseRepo.RepoPath()), err) return } + defer gitBaseRepo.Close() defer func() { ctx.JSON(200, map[string]interface{}{ @@ -978,6 +983,7 @@ func DownloadPullPatch(ctx *context.Context) { ctx.ServerError("OpenRepository", err) return } + defer headGitRepo.Close() patch, err := headGitRepo.GetFormatPatch(pr.MergeBase, pr.HeadBranch) if err != nil { diff --git a/routers/repo/release_test.go b/routers/repo/release_test.go index 524c1c7346bc2..47d1a89b54a06 100644 --- a/routers/repo/release_test.go +++ b/routers/repo/release_test.go @@ -57,5 +57,6 @@ func TestNewReleasePost(t *testing.T) { Title: testCase.Form.Title, Note: testCase.Form.Content, }, models.Cond("is_draft=?", len(testCase.Form.Draft) > 0)) + ctx.Repo.GitRepo.Close() } } diff --git a/routers/repo/setting.go b/routers/repo/setting.go index 757295069e90c..6c45afa1950a5 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -69,6 +69,11 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { // Check if repository name has been changed. if repo.LowerName != strings.ToLower(newRepoName) { isNameChanged = true + // Close the GitRepo if open + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + ctx.Repo.GitRepo = nil + } if err := models.ChangeRepositoryName(ctx.Repo.Owner, repo.Name, newRepoName); err != nil { ctx.Data["Err_RepoName"] = true switch { @@ -371,6 +376,11 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { } oldOwnerID := ctx.Repo.Owner.ID + // Close the GitRepo if open + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + ctx.Repo.GitRepo = nil + } if err = models.TransferOwnership(ctx.User, newOwner, repo); err != nil { if models.IsErrRepoAlreadyExist(err) { ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplSettingsOptions, nil) diff --git a/routers/repo/wiki.go b/routers/repo/wiki.go index 43149c034061b..0f915ba34acc6 100644 --- a/routers/repo/wiki.go +++ b/routers/repo/wiki.go @@ -144,6 +144,7 @@ func renderWikiPage(ctx *context.Context, isViewPage bool) (*git.Repository, *gi if models.IsErrWikiInvalidFileName(err) { continue } + wikiRepo.Close() ctx.ServerError("WikiFilenameToName", err) return nil, nil } else if wikiName == "_Sidebar" || wikiName == "_Footer" { @@ -171,14 +172,19 @@ func renderWikiPage(ctx *context.Context, isViewPage bool) (*git.Repository, *gi pageFilename := models.WikiNameToFilename(pageName) var entry *git.TreeEntry if entry, err = findEntryForFile(commit, pageFilename); err != nil { + wikiRepo.Close() ctx.ServerError("findEntryForFile", err) return nil, nil } else if entry == nil { + wikiRepo.Close() ctx.Redirect(ctx.Repo.RepoLink + "/wiki/_pages") return nil, nil } data := wikiContentsByEntry(ctx, entry) if ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return nil, nil } @@ -223,8 +229,16 @@ func Wiki(ctx *context.Context) { wikiRepo, entry := renderWikiPage(ctx, true) if ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return } + defer func() { + if wikiRepo != nil { + wikiRepo.Close() + } + }() if entry == nil { ctx.Data["Title"] = ctx.Tr("repo.wiki") ctx.HTML(200, tplWikiStart) @@ -260,11 +274,18 @@ func WikiPages(ctx *context.Context) { wikiRepo, commit, err := findWikiRepoCommit(ctx) if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } return } entries, err := commit.ListEntries() if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } + ctx.ServerError("ListEntries", err) return } @@ -275,6 +296,10 @@ func WikiPages(ctx *context.Context) { } c, err := wikiRepo.GetCommitByPath(entry.Name()) if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } + ctx.ServerError("GetCommit", err) return } @@ -283,6 +308,10 @@ func WikiPages(ctx *context.Context) { if models.IsErrWikiInvalidFileName(err) { continue } + if wikiRepo != nil { + wikiRepo.Close() + } + ctx.ServerError("WikiFilenameToName", err) return } @@ -294,6 +323,11 @@ func WikiPages(ctx *context.Context) { } ctx.Data["Pages"] = pages + defer func() { + if wikiRepo != nil { + wikiRepo.Close() + } + }() ctx.HTML(200, tplWikiPages) } @@ -305,6 +339,9 @@ func WikiRaw(ctx *context.Context) { return } } + defer func() { + wikiRepo.Close() + }() providedPath := ctx.Params("*") diff --git a/routers/repo/wiki_test.go b/routers/repo/wiki_test.go index 4687d24f6b28d..44fcd02035fe3 100644 --- a/routers/repo/wiki_test.go +++ b/routers/repo/wiki_test.go @@ -23,6 +23,7 @@ const message = "Wiki commit message for unit tests" func wikiEntry(t *testing.T, repo *models.Repository, wikiName string) *git.TreeEntry { wikiRepo, err := git.OpenRepository(repo.WikiPath()) assert.NoError(t, err) + defer wikiRepo.Close() commit, err := wikiRepo.GetBranchCommit("master") assert.NoError(t, err) entries, err := commit.ListEntries()