From f5a97b751883068621334f35fc9b9ae5de953b85 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 12 Jun 2026 13:35:59 +0800 Subject: [PATCH] fix: git cmd (#38084) --- modules/git/gitcmd/command.go | 11 +++++++++++ modules/gitrepo/gitrepo.go | 2 +- routers/private/hook_post_receive.go | 2 +- services/repository/files/temp_repo.go | 3 ++- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/modules/git/gitcmd/command.go b/modules/git/gitcmd/command.go index b6f56af07a9..28ed20ece0e 100644 --- a/modules/git/gitcmd/command.go +++ b/modules/git/gitcmd/command.go @@ -445,6 +445,17 @@ func (c *Command) Start(ctx context.Context) (retErr error) { c.cmd.Stdout = c.cmdStdout c.cmd.Stdin = c.cmdStdin c.cmd.Stderr = c.cmdStderr + c.cmd.Cancel = func() error { + // Golang's default cmd.Cancel only calls Process.Kill(), but here we need to close the parent pipes together: + // * for some commands like "git --batch-xxx", Windows git might have 2 processes (a wrapper and a real git process) + // * on Windows, if parent process is killed (context canceled), the children process won't be killed, and the pipe handles are still open. + // * if we don't close the parent pipes here, the children process won't exit. + // + // There is no such problem on POSIX, while it won't make things worse by closing the parent pipes also on POSIX. + err := c.cmd.Process.Kill() + c.closePipeFiles(c.parentPipeFiles) + return err + } return c.cmd.Start() } diff --git a/modules/gitrepo/gitrepo.go b/modules/gitrepo/gitrepo.go index 1af6f4406c6..17eabb2aad2 100644 --- a/modules/gitrepo/gitrepo.go +++ b/modules/gitrepo/gitrepo.go @@ -40,7 +40,7 @@ type contextKey struct { } // RepositoryFromContextOrOpen attempts to get the repository from the context or just opens it -// The caller must call "defer gitRepo.Close()" +// The caller must call Closer.Close() func RepositoryFromContextOrOpen(ctx context.Context, repo Repository) (*git.Repository, io.Closer, error) { reqCtx := reqctx.FromContext(ctx) if reqCtx != nil { diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index 3cb9eac809b..7e6e06a5f79 100644 --- a/routers/private/hook_post_receive.go +++ b/routers/private/hook_post_receive.go @@ -47,7 +47,7 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { repo *repo_model.Repository gitRepo *git.Repository ) - defer gitRepo.Close() // it's safe to call Close on a nil pointer + defer func() { _ = gitRepo.Close() }() // it's safe to call Close on a nil pointer, but it needs to use the latest value updates := make([]*repo_module.PushUpdateOptions, 0, len(opts.OldCommitIDs)) wasEmpty := false diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index 553f4232e2a..bffd3525836 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -47,7 +47,8 @@ func NewTemporaryUploadRepository(repo *repo_model.Repository) (*TemporaryUpload // Close the repository cleaning up all files func (t *TemporaryUploadRepository) Close() { - defer t.gitRepo.Close() + // must stop the repo access before removal, otherwise Windows can't remove the directory occupied by other processes + t.gitRepo.Close() if t.cleanup != nil { t.cleanup() }