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

Backport #8901 - Adjusted slightly for 1.9

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

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

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

Fixes #7947

Author: zeripath

cmd/admin.go

@@ -374,17 +374,20 @@ func runRepoSyncReleases(c *cli.Context) error {
 
 			if err = models.SyncReleasesWithTags(repo, gitRepo); err != nil {
 				log.Warn(" SyncReleasesWithTags: %v", err)
+				gitRepo.Close()
 				continue
 			}
 
 			count, err = getReleaseCount(repo.ID)
 			if err != nil {
 				log.Warn(" GetReleaseCountByRepoID: %v", err)
+				gitRepo.Close()
 				continue
 			}
 
 			log.Trace(" repo %s releases synchronized to tags: from %d to %d",
 				repo.FullName(), oldnum, count)
+			gitRepo.Close()
 		}
 	}
 

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

@@ -67,6 +67,7 @@ type Uploader interface {
 	CreateComment(issueNumber int64, comment *Comment) error
 	CreatePullRequest(pr *PullRequest) error
 	Rollback() error
+	Close()
 }
 
-```
+```

go.mod

@@ -83,8 +83,6 @@ require (
 	github.com/mattn/go-sqlite3 v1.11.0
 	github.com/mcuadros/go-version v0.0.0-20190308113854-92cdf37c5b75
 	github.com/microcosm-cc/bluemonday v0.0.0-20161012083705-f77f16ffc87a
-	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
-	github.com/modern-go/reflect2 v1.0.1 // indirect
 	github.com/mschoch/smat v0.0.0-20160514031455-90eadee771ae // indirect
 	github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc
 	github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5

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