diff --git a/modules/git/gitcmd/command.go b/modules/git/gitcmd/command.go index ee447dfd032..aa7bf7980c6 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 535d72ed98a..89061edd6d2 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 3d070a18abb..d51adfa5c43 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 5d5cc22513b..6bc1a27e17c 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() }