Add Close() method to gogitRepository (#8901) (#8956)

Backport #8901 

In investigating #7947 it has become clear that the storage component of go-git repositories needs closing.

This PR adds this Close function and adds the Close functions as necessary.

In TransferOwnership the ctx.Repo.GitRepo is closed if it is open to help prevent the risk of multiple open files.

Fixes #7947

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()
 		}
 	}
 

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()
 }
 
-```
+```

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)

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

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

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"

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"

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())

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
 

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
 

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)

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

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
 			}
 

models/pull.go

@@ -382,6 +382,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 {
@@ -571,6 +572,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 {
@@ -955,6 +957,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())
@@ -996,6 +999,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 {
@@ -1185,6 +1189,7 @@ func checkForInvalidation(requests PullRequestList, repoID int64, doer *User, br
 		if err != nil {
 			log.Error("PullRequestList.InvalidateCodeComments: %v", err)
 		}
+		gitRepo.Close()
 	}()
 	return nil
 }

models/repo.go

@@ -998,6 +998,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 {

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)

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)

models/repo_tag.go

@@ -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())
-}

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)

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")

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()
 	}
 }
 

modules/context/repo.go

@@ -427,9 +427,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
 		}
 
@@ -488,6 +497,7 @@ func RepoAssignment() macaron.Handler {
 			ctx.Data["GoDocDirectory"] = prefix + "{/dir}"
 			ctx.Data["GoDocFile"] = prefix + "{/dir}/{file}#L{line}"
 		}
+		ctx.Next()
 	}
 }
 
@@ -593,6 +603,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.
@@ -681,6 +698,8 @@ func RepoRefByType(refType RepoRefType) macaron.Handler {
 			return
 		}
 		ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount
+
+		ctx.Next()
 	}
 }
 

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)
 }

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)

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()
 	}
 }

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", &note)
@@ -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", &note)

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

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)

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 {

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()

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)

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)

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)
 

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)

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"

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)

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)

modules/migrations/base/uploader.go

@@ -17,4 +17,5 @@ type Uploader interface {
 	CreateComments(comments ...*Comment) error
 	CreatePullRequests(prs ...*PullRequest) error
 	Rollback() error
+	Close()
 }

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...)

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()

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()
 		}
 	}
 
@@ -132,8 +134,10 @@ func CommitRepoAction(opts CommitRepoActionOptions) error {
 
 			shaSum, err = gitRepo.GetBranchCommitID(refName)
 			if err != nil {
+				gitRepo.Close()
 				log.Error("GetBranchCommitID[%s]: %v", opts.RefFullName, err)
 			}
+			gitRepo.Close()
 			if err = models.PrepareWebhooks(repo, models.HookEventCreate, &api.CreatePayload{
 				Ref:     refName,
 				Sha:     shaSum,
@@ -167,8 +171,10 @@ func CommitRepoAction(opts CommitRepoActionOptions) error {
 		}
 		shaSum, err = gitRepo.GetTagCommitID(refName)
 		if err != nil {
+			gitRepo.Close()
 			log.Error("GetTagCommitID[%s]: %v", opts.RefFullName, err)
 		}
+		gitRepo.Close()
 		if err = models.PrepareWebhooks(repo, models.HookEventCreate, &api.CreatePayload{
 			Ref:     refName,
 			Sha:     shaSum,

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

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)

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,

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)

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) {

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"

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()
 

modules/repofiles/temp_repo.go

@@ -44,6 +44,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)
 	}

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{

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

modules/repofiles/update.go

@@ -429,6 +429,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)

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)

routers/api/v1/api.go

@@ -638,7 +638,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)

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 {

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)
 }

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
 	}

routers/api/v1/repo/pull.go

@@ -194,6 +194,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)
@@ -687,6 +688,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, "", ""
 	}
@@ -697,6 +699,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, "", ""
 	}
@@ -704,6 +707,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, "", ""
 	}
@@ -714,18 +718,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, "", ""
 	}

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