From 722a7c902dd39bd3d4328345ca969220640774d7 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 +- 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/graph_test.go | 1 + models/migrations/v39.go | 1 + models/migrations/v82.go | 1 + models/pull.go | 4 + models/repo.go | 1 + models/repo_activity.go | 4 + models/repo_branch.go | 4 + models/repo_sign.go | 5 + models/repo_tag.go | 24 --- models/wiki.go | 2 + models/wiki_test.go | 3 + modules/context/api.go | 9 ++ modules/context/repo.go | 19 +++ 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 | 11 ++ 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 | 3 + 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/notification/webhook/webhook.go | 2 + modules/repofiles/action.go | 2 + 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/repofiles/update.go | 1 + modules/test/context_tests.go | 1 + routers/api/v1/api.go | 2 +- routers/api/v1/repo/commits.go | 2 + 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 | 11 ++ 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 | 70 +++++++- routers/repo/wiki_test.go | 1 + services/comments/comments.go | 1 + services/gitdiff/gitdiff.go | 2 + services/mirror/mirror.go | 152 +++++++++--------- services/mirror/mirror_test.go | 1 + services/pull/commit_status.go | 1 + services/pull/pull.go | 1 + services/release/release_test.go | 1 + 75 files changed, 387 insertions(+), 102 deletions(-) delete mode 100644 models/repo_tag.go diff --git a/cmd/admin.go b/cmd/admin.go index 83ced76c966f8..1e41a0af6cc9a 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -375,17 +375,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 2511f7af8985a..0d9d8b49a77bc 100644 --- a/docs/content/doc/advanced/migrations.en-us.md +++ b/docs/content/doc/advanced/migrations.en-us.md @@ -68,6 +68,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/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 4d76ff00ceb69..eb437edf03f9a 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 bf695d4344c3f..236cb8eb30884 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 b4c535188bc66..754f64023ff6e 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 c475c70008ff1..35cb5e8b0cb18 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/graph_test.go b/models/graph_test.go index c1f0bc90d954d..78bfcb27ec6e0 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, 1) diff --git a/models/migrations/v39.go b/models/migrations/v39.go index f3b32ea873d20..dc5f6ee091f31 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 3fb4b6c59e6b8..2daa86ab07338 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 45a1daac467f9..b25c52571d77e 100644 --- a/models/pull.go +++ b/models/pull.go @@ -380,6 +380,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 { @@ -569,6 +570,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 { @@ -870,6 +872,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()) @@ -911,6 +914,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 { diff --git a/models/repo.go b/models/repo.go index ccecfe2fdf0ae..a340a391a1310 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1047,6 +1047,7 @@ func MigrateRepositoryGitData(doer, u *User, repo *Repository, opts api.MigrateR 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 aa5c2217e03e6..d25524c057826 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_sign.go b/models/repo_sign.go index bac69f76a894b..a02b027f89f89 100644 --- a/models/repo_sign.go +++ b/models/repo_sign.go @@ -149,6 +149,7 @@ func (repo *Repository) SignWikiCommit(u *User) (bool, string) { if err != nil { return false, "" } + defer gitRepo.Close() commit, err := gitRepo.GetCommit("HEAD") if err != nil { return false, "" @@ -194,6 +195,7 @@ func (repo *Repository) SignCRUDAction(u *User, tmpBasePath, parentCommit string if err != nil { return false, "" } + defer gitRepo.Close() commit, err := gitRepo.GetCommit(parentCommit) if err != nil { return false, "" @@ -242,6 +244,7 @@ func (repo *Repository) SignMerge(u *User, tmpBasePath, baseCommit, headCommit s if err != nil { return false, "" } + defer gitRepo.Close() } commit, err := gitRepo.GetCommit(baseCommit) if err != nil { @@ -257,6 +260,7 @@ func (repo *Repository) SignMerge(u *User, tmpBasePath, baseCommit, headCommit s if err != nil { return false, "" } + defer gitRepo.Close() } commit, err := gitRepo.GetCommit(headCommit) if err != nil { @@ -272,6 +276,7 @@ func (repo *Repository) SignMerge(u *User, tmpBasePath, baseCommit, headCommit s if err != nil { return false, "" } + defer gitRepo.Close() } commit, err := gitRepo.GetCommit(headCommit) 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/wiki.go b/models/wiki.go index 858fe1d6d04a9..8b63716afa1f0 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 { @@ -283,6 +284,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 024ae487f1156..c1de37dd21a09 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -186,7 +186,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 bd3456773fa1a..f41505e7acf75 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -454,9 +454,18 @@ func RepoAssignment() macaron.Handler { } 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() + } + }() + // Stop at this point when the repo is empty. if ctx.Repo.Repository.IsEmpty { ctx.Data["BranchName"] = ctx.Repo.Repository.DefaultBranch + ctx.Next() return } @@ -515,6 +524,7 @@ func RepoAssignment() macaron.Handler { ctx.Data["GoDocDirectory"] = prefix + "{/dir}" ctx.Data["GoDocFile"] = prefix + "{/dir}/{file}#L{line}" } + ctx.Next() } } @@ -636,6 +646,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. @@ -724,6 +741,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 80f61097724e6..ffca2524be1fe 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" @@ -122,6 +123,16 @@ 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 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 6d8ee6453f5a1..87dd6763b3944 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 { @@ -41,6 +42,7 @@ 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) @@ -54,6 +56,7 @@ 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) 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 6fbcb7ac13c0f..bc1f6a566262b 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") assert.NoError(t, err) 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 a3a9c9fac619b..ae1be84b88a97 100644 --- a/modules/migrations/base/uploader.go +++ b/modules/migrations/base/uploader.go @@ -17,4 +17,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 676667b426aad..81a6116a237d1 100644 --- a/modules/migrations/gitea.go +++ b/modules/migrations/gitea.go @@ -131,6 +131,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() + } +} + // CreateTopics creates topics func (g *GiteaLocalUploader) CreateTopics(topics ...string) error { return models.SaveTopics(g.repo.ID, topics...) diff --git a/modules/migrations/migrate.go b/modules/migrations/migrate.go index bbc1dc2d56107..7a5071e1258dd 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -94,6 +94,7 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts if err := uploader.CreateRepo(repo, opts); err != nil { return err } + defer uploader.Close() log.Trace("migrating topics") topics, err := downloader.GetTopics() diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go index 27afbba7ab81e..43be0d2e1cb40 100644 --- a/modules/notification/webhook/webhook.go +++ b/modules/notification/webhook/webhook.go @@ -575,9 +575,11 @@ func (m *webhookNotifier) NotifyCreateRef(pusher *models.User, repo *models.Repo shaSum, err := gitRepo.GetRefCommitID(refFullName) if err != nil { + gitRepo.Close() log.Error("GetRefCommitID[%s]: %v", refFullName, err) return } + gitRepo.Close() if err = webhook_module.PrepareWebhooks(repo, models.HookEventCreate, &api.CreatePayload{ Ref: refName, diff --git a/modules/repofiles/action.go b/modules/repofiles/action.go index 996363863d9a4..8b35cba7260ee 100644 --- a/modules/repofiles/action.go +++ b/modules/repofiles/action.go @@ -53,9 +53,11 @@ func CommitRepoAction(opts CommitRepoActionOptions) error { } if err := gitRepo.SetDefaultBranch(repo.DefaultBranch); err != nil { if !git.IsErrUnsupportedVersion(err) { + gitRepo.Close() return err } } + gitRepo.Close() } } 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 de5ed1d754853..db2c7552c4ffe 100644 --- a/modules/repofiles/diff_test.go +++ b/modules/repofiles/diff_test.go @@ -22,6 +22,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" @@ -119,6 +121,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 95ec175ed47fd..3cb4eb472bf0f 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 abc224c2c295b..6bd775d9d22f8 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/repofiles/update.go b/modules/repofiles/update.go index 5479616c4b814..ef56609f4d05a 100644 --- a/modules/repofiles/update.go +++ b/modules/repofiles/update.go @@ -430,6 +430,7 @@ func PushUpdate(repo *models.Repository, branch string, opts models.PushUpdateOp if err != nil { return 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/modules/test/context_tests.go b/modules/test/context_tests.go index 92df1c576252b..cf9c5fbc548ad 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 b68717f7c8751..6fdd0d074c902 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -641,7 +641,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 0156aaaa05cd3..163a06a95e8e1 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -51,6 +51,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) @@ -113,6 +114,7 @@ func GetAllCommits(ctx *context.APIContext) { ctx.ServerError("OpenRepository", err) return } + defer gitRepo.Close() page := ctx.QueryInt("page") if page <= 0 { diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index f1a61bb0be04d..175235c5ef374 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 6af1ba1b04b0c..9abcaa0496a15 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -195,6 +195,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) @@ -722,6 +723,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, "", "" } @@ -732,6 +734,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, "", "" } @@ -739,6 +742,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, "", "" } @@ -749,18 +753,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 6cfdb461ee261..0a764113ab805 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 b9e14abfb87bb..e45f0464353c8 100644 --- a/routers/repo/compare.go +++ b/routers/repo/compare.go @@ -157,6 +157,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, "", "" } @@ -167,6 +168,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * baseRepo, permBase) } + headGitRepo.Close() ctx.NotFound("ParseCompareInfo", nil) return nil, nil, nil, nil, "", "" } @@ -174,6 +176,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, "", "" } @@ -184,6 +187,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * headRepo, permHead) } + headGitRepo.Close() ctx.NotFound("ParseCompareInfo", nil) return nil, nil, nil, nil, "", "" } @@ -199,6 +203,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, "", "" } @@ -219,12 +224,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, "", "" } @@ -356,6 +363,8 @@ func parseBaseRepoInfo(ctx *context.Context, repo *models.Repository) error { if err != nil { return err } + defer baseGitRepo.Close() + ctx.Data["BaseRepoBranches"], err = baseGitRepo.GetBranches() if err != nil { return err @@ -369,6 +378,8 @@ func CompareDiff(ctx *context.Context) { if ctx.Written() { return } + defer headGitRepo.Close() + if err := parseBaseRepoInfo(ctx, headRepo); err != nil { ctx.ServerError("parseBaseRepoInfo", err) return diff --git a/routers/repo/editor_test.go b/routers/repo/editor_test.go index ca00be74b7fec..ec7aee1e77ff9 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 8269717e57374..67849f33e12f4 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -352,6 +352,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) @@ -534,6 +535,7 @@ func ViewPullFiles(ctx *context.Context) { ctx.ServerError("OpenRepository", err) return } + defer headGitRepo.Close() headCommitID, err := headGitRepo.GetBranchCommitID(pull.HeadBranch) if err != nil { @@ -748,6 +750,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() { @@ -913,12 +916,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{}{ @@ -1047,6 +1052,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 f699c1d685641..fa215357d2d70 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -73,6 +73,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 { @@ -379,6 +384,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 02fbe4a1ddad6..6cf194365891f 100644 --- a/routers/repo/wiki.go +++ b/routers/repo/wiki.go @@ -146,6 +146,9 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { // Get page list. entries, err := commit.ListEntries() if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } ctx.ServerError("ListEntries", err) return nil, nil } @@ -159,6 +162,9 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { if models.IsErrWikiInvalidFileName(err) { continue } + if wikiRepo != nil { + wikiRepo.Close() + } ctx.ServerError("WikiFilenameToName", err) return nil, nil } else if wikiName == "_Sidebar" || wikiName == "_Footer" { @@ -188,16 +194,25 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { ctx.Redirect(ctx.Repo.RepoLink + "/wiki/_pages") } if entry == nil || ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return nil, nil } sidebarContent, _, _, _ := wikiContentsByName(ctx, commit, "_Sidebar") if ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return nil, nil } footerContent, _, _, _ := wikiContentsByName(ctx, commit, "_Footer") if ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return nil, nil } @@ -218,6 +233,9 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { wikiRepo, commit, err := findWikiRepoCommit(ctx) if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } if !git.IsErrNotExist(err) { ctx.ServerError("GetBranchCommit", err) } @@ -241,6 +259,9 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) ctx.Redirect(ctx.Repo.RepoLink + "/wiki/_pages") } if entry == nil || ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return nil, nil } @@ -263,6 +284,9 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) // get Commit Count commitsHistory, err := wikiRepo.CommitsByFileAndRangeNoFollow("master", pageFilename, page) if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } ctx.ServerError("CommitsByFileAndRangeNoFollow", err) return nil, nil } @@ -279,13 +303,21 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) } func renderEditPage(ctx *context.Context) { - _, commit, err := findWikiRepoCommit(ctx) + wikiRepo, commit, err := findWikiRepoCommit(ctx) if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } if !git.IsErrNotExist(err) { ctx.ServerError("GetBranchCommit", err) } return } + defer func() { + if wikiRepo != nil { + wikiRepo.Close() + } + }() // get requested pagename pageName := models.NormalizeWikiName(ctx.Params(":page")) @@ -327,8 +359,16 @@ func Wiki(ctx *context.Context) { wikiRepo, entry := renderViewPage(ctx) 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) @@ -364,8 +404,16 @@ func WikiRevision(ctx *context.Context) { wikiRepo, entry := renderRevisionPage(ctx) 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) @@ -397,11 +445,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 } @@ -412,6 +467,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 } @@ -420,6 +479,10 @@ func WikiPages(ctx *context.Context) { if models.IsErrWikiInvalidFileName(err) { continue } + if wikiRepo != nil { + wikiRepo.Close() + } + ctx.ServerError("WikiFilenameToName", err) return } @@ -431,6 +494,11 @@ func WikiPages(ctx *context.Context) { } ctx.Data["Pages"] = pages + defer func() { + if wikiRepo != nil { + wikiRepo.Close() + } + }() ctx.HTML(200, tplWikiPages) } 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() diff --git a/services/comments/comments.go b/services/comments/comments.go index 1ae5e2743fe7f..ba40bf582d30b 100644 --- a/services/comments/comments.go +++ b/services/comments/comments.go @@ -49,6 +49,7 @@ func CreateCodeComment(doer *models.User, repo *models.Repository, issue *models 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/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index c2c5675d9fd21..9c2aef5c97ce1 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -678,6 +678,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID if err != nil { return nil, err } + defer gitRepo.Close() commit, err := gitRepo.GetCommit(afterCommitID) if err != nil { @@ -750,6 +751,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/services/mirror/mirror.go b/services/mirror/mirror.go index 11430c20700de..8c8131b5c2b28 100644 --- a/services/mirror/mirror.go +++ b/services/mirror/mirror.go @@ -197,8 +197,10 @@ func runSync(m *models.Mirror) ([]*mirrorSyncResult, bool) { return nil, false } if err = models.SyncReleasesWithTags(m.Repo, gitRepo); err != nil { + gitRepo.Close() 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) @@ -290,97 +292,103 @@ func Update() { func SyncMirrors() { // Start listening on new sync requests. for repoID := range mirrorQueue.Queue() { - log.Trace("SyncMirrors [repo_id: %v]", repoID) - mirrorQueue.Remove(repoID) + syncMirror(repoID) + } +} + +func syncMirror(repoID string) { + log.Trace("SyncMirrors [repo_id: %v]", repoID) + mirrorQueue.Remove(repoID) + + m, err := models.GetMirrorByRepoID(com.StrTo(repoID).MustInt64()) + if err != nil { + log.Error("GetMirrorByRepoID [%s]: %v", repoID, err) + return + + } + + results, ok := runSync(m) + if !ok { + return + } + + m.ScheduleNextUpdate() + if err = models.UpdateMirror(m); err != nil { + log.Error("UpdateMirror [%s]: %v", repoID, err) + return + } - m, err := models.GetMirrorByRepoID(com.StrTo(repoID).MustInt64()) + var gitRepo *git.Repository + if len(results) == 0 { + log.Trace("SyncMirrors [repo_id: %d]: no commits fetched", m.RepoID) + } else { + gitRepo, err = git.OpenRepository(m.Repo.RepoPath()) if err != nil { - log.Error("GetMirrorByRepoID [%s]: %v", repoID, err) - continue + log.Error("OpenRepository [%d]: %v", m.RepoID, err) + return } + defer gitRepo.Close() + } - results, ok := runSync(m) - if !ok { + for _, result := range results { + // Discard GitHub pull requests, i.e. refs/pull/* + if strings.HasPrefix(result.refName, "refs/pull/") { continue } - m.ScheduleNextUpdate() - if err = models.UpdateMirror(m); err != nil { - log.Error("UpdateMirror [%s]: %v", repoID, err) + // Create reference + if result.oldCommitID == gitShortEmptySha { + if err = SyncCreateAction(m.Repo, result.refName); err != nil { + log.Error("SyncCreateAction [repo_id: %d]: %v", m.RepoID, err) + } continue } - var gitRepo *git.Repository - if len(results) == 0 { - log.Trace("SyncMirrors [repo_id: %d]: no commits fetched", m.RepoID) - } else { - gitRepo, err = git.OpenRepository(m.Repo.RepoPath()) - if err != nil { - log.Error("OpenRepository [%d]: %v", m.RepoID, err) - continue + // Delete reference + if result.newCommitID == gitShortEmptySha { + if err = SyncDeleteAction(m.Repo, result.refName); err != nil { + log.Error("SyncDeleteAction [repo_id: %d]: %v", m.RepoID, err) } + continue } - for _, result := range results { - // Discard GitHub pull requests, i.e. refs/pull/* - if strings.HasPrefix(result.refName, "refs/pull/") { - continue - } - - // Create reference - if result.oldCommitID == gitShortEmptySha { - if err = SyncCreateAction(m.Repo, result.refName); err != nil { - log.Error("SyncCreateAction [repo_id: %d]: %v", m.RepoID, err) - } - continue - } - - // Delete reference - if result.newCommitID == gitShortEmptySha { - if err = SyncDeleteAction(m.Repo, result.refName); err != nil { - log.Error("SyncDeleteAction [repo_id: %d]: %v", m.RepoID, err) - } - continue - } - - // Push commits - oldCommitID, err := git.GetFullCommitID(gitRepo.Path, result.oldCommitID) - if err != nil { - log.Error("GetFullCommitID [%d]: %v", m.RepoID, err) - continue - } - newCommitID, err := git.GetFullCommitID(gitRepo.Path, result.newCommitID) - if err != nil { - log.Error("GetFullCommitID [%d]: %v", m.RepoID, err) - continue - } - commits, err := gitRepo.CommitsBetweenIDs(newCommitID, oldCommitID) - if err != nil { - log.Error("CommitsBetweenIDs [repo_id: %d, new_commit_id: %s, old_commit_id: %s]: %v", m.RepoID, newCommitID, oldCommitID, err) - continue - } - if err = SyncPushAction(m.Repo, SyncPushActionOptions{ - RefName: result.refName, - OldCommitID: oldCommitID, - NewCommitID: newCommitID, - Commits: models.ListToPushCommits(commits), - }); err != nil { - log.Error("SyncPushAction [repo_id: %d]: %v", m.RepoID, err) - continue - } + // Push commits + oldCommitID, err := git.GetFullCommitID(gitRepo.Path, result.oldCommitID) + if err != nil { + log.Error("GetFullCommitID [%d]: %v", m.RepoID, err) + continue } - - // Get latest commit date and update to current repository updated time - commitDate, err := git.GetLatestCommitTime(m.Repo.RepoPath()) + newCommitID, err := git.GetFullCommitID(gitRepo.Path, result.newCommitID) if err != nil { - log.Error("GetLatestCommitDate [%d]: %v", m.RepoID, err) + log.Error("GetFullCommitID [%d]: %v", m.RepoID, err) continue } - - if err = models.UpdateRepositoryUpdatedTime(m.RepoID, commitDate); err != nil { - log.Error("Update repository 'updated_unix' [%d]: %v", m.RepoID, err) + commits, err := gitRepo.CommitsBetweenIDs(newCommitID, oldCommitID) + if err != nil { + log.Error("CommitsBetweenIDs [repo_id: %d, new_commit_id: %s, old_commit_id: %s]: %v", m.RepoID, newCommitID, oldCommitID, err) continue } + if err = SyncPushAction(m.Repo, SyncPushActionOptions{ + RefName: result.refName, + OldCommitID: oldCommitID, + NewCommitID: newCommitID, + Commits: models.ListToPushCommits(commits), + }); err != nil { + log.Error("SyncPushAction [repo_id: %d]: %v", m.RepoID, err) + continue + } + } + + // Get latest commit date and update to current repository updated time + commitDate, err := git.GetLatestCommitTime(m.Repo.RepoPath()) + if err != nil { + log.Error("GetLatestCommitDate [%d]: %v", m.RepoID, err) + return + } + + if err = models.UpdateRepositoryUpdatedTime(m.RepoID, commitDate); err != nil { + log.Error("Update repository 'updated_unix' [%d]: %v", m.RepoID, err) + return } } diff --git a/services/mirror/mirror_test.go b/services/mirror/mirror_test.go index 9ad11b726563c..37e8a7be57376 100644 --- a/services/mirror/mirror_test.go +++ b/services/mirror/mirror_test.go @@ -51,6 +51,7 @@ func TestRelease_MirrorDelete(t *testing.T) { gitRepo, err := git.OpenRepository(repoPath) assert.NoError(t, err) + defer gitRepo.Close() findOptions := models.FindReleasesOptions{IncludeDrafts: true, IncludeTags: true} initCount, err := models.GetReleaseCountByRepoID(mirror.ID, findOptions) diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go index 2872db7bd2896..ca00cdaad9bc4 100644 --- a/services/pull/commit_status.go +++ b/services/pull/commit_status.go @@ -55,6 +55,7 @@ func IsPullCommitStatusPass(pr *models.PullRequest) (bool, error) { if err != nil { return false, errors.Wrap(err, "OpenRepository") } + defer headGitRepo.Close() if !headGitRepo.IsBranchExist(pr.HeadBranch) { return false, errors.New("Head branch does not exist, can not merge") diff --git a/services/pull/pull.go b/services/pull/pull.go index 4e981b2b260c9..7a9c2ef9ad03d 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -48,6 +48,7 @@ func checkForInvalidation(requests models.PullRequestList, repoID int64, doer *m if err != nil { log.Error("PullRequestList.InvalidateCodeComments: %v", err) } + gitRepo.Close() }() return nil } diff --git a/services/release/release_test.go b/services/release/release_test.go index d30dfee2865cd..effab21251062 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -27,6 +27,7 @@ func TestRelease_Create(t *testing.T) { gitRepo, err := git.OpenRepository(repoPath) assert.NoError(t, err) + defer gitRepo.Close() assert.NoError(t, CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID,