From 197a1cf9060089685addef5d9ea225a5278af543 Mon Sep 17 00:00:00 2001
From: Sebastiaan Dammann <sebastiaandammann@outlook.com>
Date: Sun, 10 Nov 2019 11:37:37 +0100
Subject: [PATCH] Close resources left by go-git after request is finished and
 before actions that touch the repository directory

Especially Windows doesn't like it when there are open handles when a directory is moved or copied.
---
 modules/context/context.go | 18 ++++++++++++++++++
 modules/git/repo.go        |  5 +++++
 routers/repo/setting.go    | 13 +++++++++++++
 routers/routes/routes.go   |  2 ++
 4 files changed, 38 insertions(+)

diff --git a/modules/context/context.go b/modules/context/context.go
index 067ef0b59cf20..196ae818c90b7 100644
--- a/modules/context/context.go
+++ b/modules/context/context.go
@@ -46,6 +46,15 @@ type Context struct {
 	Org  *Organization
 }
 
+// Close Releases resources used by this context, like file descriptors
+func (ctx *Context) Close() {
+	if ctx.Repo != nil && ctx.Repo.GitRepo != nil {
+		if err := ctx.Repo.GitRepo.Close(); err != nil {
+			log.Warn("Unable to clean up repository resources %s", err.Error())
+		}
+	}
+}
+
 // IsUserSiteAdmin returns true if current user is a site admin
 func (ctx *Context) IsUserSiteAdmin() bool {
 	return ctx.IsSigned && ctx.User.IsAdmin
@@ -342,3 +351,12 @@ func Contexter() macaron.Handler {
 		c.Map(ctx)
 	}
 }
+
+// Cleanup Cleans up used resources like open file descriptors at the end of the request
+func Cleanup() macaron.Handler {
+	return func(ctx *Context) {
+		defer ctx.Close()
+
+		ctx.Next()
+	}
+}
diff --git a/modules/git/repo.go b/modules/git/repo.go
index 28e54a1bbcf59..802c2c6d7090c 100644
--- a/modules/git/repo.go
+++ b/modules/git/repo.go
@@ -107,6 +107,11 @@ func OpenRepository(repoPath string) (*Repository, error) {
 	}, nil
 }
 
+// Close Release file descriptors that are left open for performance reasons
+func (repo *Repository) Close() error {
+	return repo.gogitStorage.Close()
+}
+
 // IsEmpty Check if repository is empty.
 func (repo *Repository) IsEmpty() (bool, error) {
 	var errbuf strings.Builder
diff --git a/routers/repo/setting.go b/routers/repo/setting.go
index 757295069e90c..f3d54ccce7d51 100644
--- a/routers/repo/setting.go
+++ b/routers/repo/setting.go
@@ -69,6 +69,13 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) {
 		// Check if repository name has been changed.
 		if repo.LowerName != strings.ToLower(newRepoName) {
 			isNameChanged = true
+
+			// Close any file descriptors, primarily for Windows which refuses to move directories with open descriptors
+			if err := ctx.Repo.GitRepo.Close(); err != nil {
+				ctx.ServerError("ChangeRepositoryName", err)
+				return
+			}
+
 			if err := models.ChangeRepositoryName(ctx.Repo.Owner, repo.Name, newRepoName); err != nil {
 				ctx.Data["Err_RepoName"] = true
 				switch {
@@ -370,6 +377,12 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) {
 			return
 		}
 
+		// Close any file descriptors, primarily for Windows which refuses to move directories with open descriptors
+		if err := ctx.Repo.GitRepo.Close(); err != nil {
+			ctx.ServerError("ChangeRepositoryName", err)
+			return
+		}
+
 		oldOwnerID := ctx.Repo.Owner.ID
 		if err = models.TransferOwnership(ctx.User, newOwner, repo); err != nil {
 			if models.IsErrRepoAlreadyExist(err) {
diff --git a/routers/routes/routes.go b/routers/routes/routes.go
index a5206682f5c34..28602e7ee6a7a 100644
--- a/routers/routes/routes.go
+++ b/routers/routes/routes.go
@@ -220,6 +220,8 @@ func NewMacaron() *macaron.Macaron {
 	// OK we are now set-up enough to allow us to create a nicer recovery than
 	// the default macaron recovery
 	m.Use(context.Recovery())
+
+	m.Use(context.Cleanup())
 	m.SetAutoHead(true)
 	return m
 }