Skip to content

Commit 60c5339

Browse files
zeripathlafriks
authored andcommitted
Graceful: Cancel Process on monitor pages & HammerTime (#9213)
* Graceful: Create callbacks to with contexts * Graceful: Say when Gitea is completely finished * Graceful: Git and Process within HammerTime Force all git commands to terminate at HammerTime Force all process commands to terminate at HammerTime Move almost all git processes to run as git Commands * Graceful: Always Hammer after Shutdown * ProcessManager: Add cancel functionality * Fix tests * Make sure that process.Manager.Kill() cancels * Make threadsafe access to Processes and remove own unused Kill * Remove cmd from the process manager as it is no longer used * the default context is the correct context * get rid of double till
1 parent 8f8c250 commit 60c5339

File tree

21 files changed

+535
-197
lines changed

21 files changed

+535
-197
lines changed

cmd/web.go

+1
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ func runWeb(ctx *cli.Context) error {
197197
log.Info("HTTP Listener: %s Closed", listenAddr)
198198
graceful.Manager.WaitForServers()
199199
graceful.Manager.WaitForTerminate()
200+
log.Info("PID: %d Gitea Web Finished", os.Getpid())
200201
log.Close()
201202
return nil
202203
}

models/pull.go

+14-17
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818

1919
"code.gitea.io/gitea/modules/git"
2020
"code.gitea.io/gitea/modules/log"
21-
"code.gitea.io/gitea/modules/process"
2221
"code.gitea.io/gitea/modules/setting"
2322
api "code.gitea.io/gitea/modules/structs"
2423
"code.gitea.io/gitea/modules/sync"
@@ -536,16 +535,13 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) {
536535
headFile := pr.GetGitRefName()
537536

538537
// Check if a pull request is merged into BaseBranch
539-
_, stderr, err := process.GetManager().ExecDirEnv(-1, "", fmt.Sprintf("isMerged (git merge-base --is-ancestor): %d", pr.BaseRepo.ID),
540-
[]string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()},
541-
git.GitExecutable, "merge-base", "--is-ancestor", headFile, pr.BaseBranch)
542-
538+
_, err := git.NewCommand("merge-base", "--is-ancestor", headFile, pr.BaseBranch).RunInDirWithEnv(pr.BaseRepo.RepoPath(), []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()})
543539
if err != nil {
544540
// Errors are signaled by a non-zero status that is not 1
545541
if strings.Contains(err.Error(), "exit status 1") {
546542
return nil, nil
547543
}
548-
return nil, fmt.Errorf("git merge-base --is-ancestor: %v %v", stderr, err)
544+
return nil, fmt.Errorf("git merge-base --is-ancestor: %v", err)
549545
}
550546

551547
commitIDBytes, err := ioutil.ReadFile(pr.BaseRepo.RepoPath() + "/" + headFile)
@@ -559,11 +555,9 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) {
559555
cmd := commitID[:40] + ".." + pr.BaseBranch
560556

561557
// Get the commit from BaseBranch where the pull request got merged
562-
mergeCommit, stderr, err := process.GetManager().ExecDirEnv(-1, "", fmt.Sprintf("isMerged (git rev-list --ancestry-path --merges --reverse): %d", pr.BaseRepo.ID),
563-
[]string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()},
564-
git.GitExecutable, "rev-list", "--ancestry-path", "--merges", "--reverse", cmd)
558+
mergeCommit, err := git.NewCommand("rev-list", "--ancestry-path", "--merges", "--reverse", cmd).RunInDirWithEnv("", []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()})
565559
if err != nil {
566-
return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %v %v", stderr, err)
560+
return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %v", err)
567561
} else if len(mergeCommit) < 40 {
568562
// PR was fast-forwarded, so just use last commit of PR
569563
mergeCommit = commitID[:40]
@@ -621,12 +615,9 @@ func (pr *PullRequest) testPatch(e Engine) (err error) {
621615
indexTmpPath := filepath.Join(os.TempDir(), "gitea-"+pr.BaseRepo.Name+"-"+strconv.Itoa(time.Now().Nanosecond()))
622616
defer os.Remove(indexTmpPath)
623617

624-
var stderr string
625-
_, stderr, err = process.GetManager().ExecDirEnv(-1, "", fmt.Sprintf("testPatch (git read-tree): %d", pr.BaseRepo.ID),
626-
[]string{"GIT_DIR=" + pr.BaseRepo.RepoPath(), "GIT_INDEX_FILE=" + indexTmpPath},
627-
git.GitExecutable, "read-tree", pr.BaseBranch)
618+
_, err = git.NewCommand("read-tree", pr.BaseBranch).RunInDirWithEnv("", []string{"GIT_DIR=" + pr.BaseRepo.RepoPath(), "GIT_INDEX_FILE=" + indexTmpPath})
628619
if err != nil {
629-
return fmt.Errorf("git read-tree --index-output=%s %s: %v - %s", indexTmpPath, pr.BaseBranch, err, stderr)
620+
return fmt.Errorf("git read-tree --index-output=%s %s: %v", indexTmpPath, pr.BaseBranch, err)
630621
}
631622

632623
prUnit, err := pr.BaseRepo.getUnit(e, UnitTypePullRequests)
@@ -642,9 +633,15 @@ func (pr *PullRequest) testPatch(e Engine) (err error) {
642633
args = append(args, patchPath)
643634
pr.ConflictedFiles = []string{}
644635

645-
_, stderr, err = process.GetManager().ExecDirEnv(-1, "", fmt.Sprintf("testPatch (git apply --check): %d", pr.BaseRepo.ID),
636+
stderrBuilder := new(strings.Builder)
637+
err = git.NewCommand(args...).RunInDirTimeoutEnvPipeline(
646638
[]string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()},
647-
git.GitExecutable, args...)
639+
-1,
640+
"",
641+
nil,
642+
stderrBuilder)
643+
stderr := stderrBuilder.String()
644+
648645
if err != nil {
649646
for i := range patchConflicts {
650647
if strings.Contains(stderr, patchConflicts[i]) {

models/repo.go

+46-46
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
"code.gitea.io/gitea/modules/log"
3131
"code.gitea.io/gitea/modules/markup"
3232
"code.gitea.io/gitea/modules/options"
33-
"code.gitea.io/gitea/modules/process"
3433
"code.gitea.io/gitea/modules/setting"
3534
"code.gitea.io/gitea/modules/structs"
3635
api "code.gitea.io/gitea/modules/structs"
@@ -1202,11 +1201,11 @@ func initRepoCommit(tmpPath string, u *User) (err error) {
12021201
"GIT_COMMITTER_DATE="+commitTimeStr,
12031202
)
12041203

1205-
var stderr string
1206-
if _, stderr, err = process.GetManager().ExecDir(-1,
1207-
tmpPath, fmt.Sprintf("initRepoCommit (git add): %s", tmpPath),
1208-
git.GitExecutable, "add", "--all"); err != nil {
1209-
return fmt.Errorf("git add: %s", stderr)
1204+
if stdout, err := git.NewCommand("add", "--all").
1205+
SetDescription(fmt.Sprintf("initRepoCommit (git add): %s", tmpPath)).
1206+
RunInDir(tmpPath); err != nil {
1207+
log.Error("git add --all failed: Stdout: %s\nError: %v", stdout, err)
1208+
return fmt.Errorf("git add --all: %v", err)
12101209
}
12111210

12121211
binVersion, err := git.BinVersion()
@@ -1228,18 +1227,20 @@ func initRepoCommit(tmpPath string, u *User) (err error) {
12281227
}
12291228
}
12301229

1231-
if _, stderr, err = process.GetManager().ExecDirEnv(-1,
1232-
tmpPath, fmt.Sprintf("initRepoCommit (git commit): %s", tmpPath),
1233-
env,
1234-
git.GitExecutable, args...); err != nil {
1235-
return fmt.Errorf("git commit: %s", stderr)
1230+
if stdout, err := git.NewCommand(args...).
1231+
SetDescription(fmt.Sprintf("initRepoCommit (git commit): %s", tmpPath)).
1232+
RunInDirWithEnv(tmpPath, env); err != nil {
1233+
log.Error("Failed to commit: %v: Stdout: %s\nError: %v", args, stdout, err)
1234+
return fmt.Errorf("git commit: %v", err)
12361235
}
12371236

1238-
if _, stderr, err = process.GetManager().ExecDir(-1,
1239-
tmpPath, fmt.Sprintf("initRepoCommit (git push): %s", tmpPath),
1240-
git.GitExecutable, "push", "origin", "master"); err != nil {
1241-
return fmt.Errorf("git push: %s", stderr)
1237+
if stdout, err := git.NewCommand("push", "origin", "master").
1238+
SetDescription(fmt.Sprintf("initRepoCommit (git push): %s", tmpPath)).
1239+
RunInDir(tmpPath); err != nil {
1240+
log.Error("Failed to push back to master: Stdout: %s\nError: %v", stdout, err)
1241+
return fmt.Errorf("git push: %v", err)
12421242
}
1243+
12431244
return nil
12441245
}
12451246

@@ -1297,14 +1298,11 @@ func prepareRepoCommit(e Engine, repo *Repository, tmpDir, repoPath string, opts
12971298
)
12981299

12991300
// Clone to temporary path and do the init commit.
1300-
_, stderr, err := process.GetManager().ExecDirEnv(
1301-
-1, "",
1302-
fmt.Sprintf("initRepository(git clone): %s", repoPath),
1303-
env,
1304-
git.GitExecutable, "clone", repoPath, tmpDir,
1305-
)
1306-
if err != nil {
1307-
return fmt.Errorf("git clone: %v - %s", err, stderr)
1301+
if stdout, err := git.NewCommand("clone", repoPath, tmpDir).
1302+
SetDescription(fmt.Sprintf("initRepository (git clone): %s to %s", repoPath, tmpDir)).
1303+
RunInDirWithEnv("", env); err != nil {
1304+
log.Error("Failed to clone from %v into %s: stdout: %s\nError: %v", repo, tmpDir, stdout, err)
1305+
return fmt.Errorf("git clone: %v", err)
13081306
}
13091307

13101308
// README
@@ -1584,11 +1582,11 @@ func CreateRepository(doer, u *User, opts CreateRepoOptions) (_ *Repository, err
15841582
}
15851583
}
15861584

1587-
_, stderr, err := process.GetManager().ExecDir(-1,
1588-
repoPath, fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath),
1589-
git.GitExecutable, "update-server-info")
1590-
if err != nil {
1591-
return nil, errors.New("CreateRepository(git update-server-info): " + stderr)
1585+
if stdout, err := git.NewCommand("update-server-info").
1586+
SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)).
1587+
RunInDir(repoPath); err != nil {
1588+
log.Error("CreateRepitory(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)
1589+
return nil, fmt.Errorf("CreateRepository(git update-server-info): %v", err)
15921590
}
15931591
}
15941592

@@ -2422,12 +2420,13 @@ func GitGcRepos() error {
24222420
if err := repo.GetOwner(); err != nil {
24232421
return err
24242422
}
2425-
_, stderr, err := process.GetManager().ExecDir(
2426-
time.Duration(setting.Git.Timeout.GC)*time.Second,
2427-
RepoPath(repo.Owner.Name, repo.Name), "Repository garbage collection",
2428-
git.GitExecutable, args...)
2429-
if err != nil {
2430-
return fmt.Errorf("%v: %v", err, stderr)
2423+
if stdout, err := git.NewCommand(args...).
2424+
SetDescription(fmt.Sprintf("Repository Garbage Collection: %s", repo.FullName())).
2425+
RunInDirTimeout(
2426+
time.Duration(setting.Git.Timeout.GC)*time.Second,
2427+
RepoPath(repo.Owner.Name, repo.Name)); err != nil {
2428+
log.Error("Repository garbage collection failed for %v. Stdout: %s\nError: %v", repo, stdout, err)
2429+
return fmt.Errorf("Repository garbage collection failed: Error: %v", err)
24312430
}
24322431
return nil
24332432
})
@@ -2647,18 +2646,19 @@ func ForkRepository(doer, owner *User, oldRepo *Repository, name, desc string) (
26472646
}
26482647

26492648
repoPath := RepoPath(owner.Name, repo.Name)
2650-
_, stderr, err := process.GetManager().ExecTimeout(10*time.Minute,
2651-
fmt.Sprintf("ForkRepository(git clone): %s/%s", owner.Name, repo.Name),
2652-
git.GitExecutable, "clone", "--bare", oldRepo.repoPath(sess), repoPath)
2653-
if err != nil {
2654-
return nil, fmt.Errorf("git clone: %v", stderr)
2655-
}
2656-
2657-
_, stderr, err = process.GetManager().ExecDir(-1,
2658-
repoPath, fmt.Sprintf("ForkRepository(git update-server-info): %s", repoPath),
2659-
git.GitExecutable, "update-server-info")
2660-
if err != nil {
2661-
return nil, fmt.Errorf("git update-server-info: %v", stderr)
2649+
if stdout, err := git.NewCommand(
2650+
"clone", "--bare", oldRepo.repoPath(sess), repoPath).
2651+
SetDescription(fmt.Sprintf("ForkRepository(git clone): %s to %s", oldRepo.FullName(), repo.FullName())).
2652+
RunInDirTimeout(10*time.Minute, ""); err != nil {
2653+
log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, oldRepo, stdout, err)
2654+
return nil, fmt.Errorf("git clone: %v", err)
2655+
}
2656+
2657+
if stdout, err := git.NewCommand("update-server-info").
2658+
SetDescription(fmt.Sprintf("ForkRepository(git update-server-info): %s", repo.FullName())).
2659+
RunInDir(repoPath); err != nil {
2660+
log.Error("Fork Repository (git update-server-info) failed for %v:\nStdout: %s\nError: %v", repo, stdout, err)
2661+
return nil, fmt.Errorf("git update-server-info: %v", err)
26622662
}
26632663

26642664
if err = createDelegateHooks(repoPath); err != nil {

models/repo_generate.go

+5-9
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616

1717
"code.gitea.io/gitea/modules/git"
1818
"code.gitea.io/gitea/modules/log"
19-
"code.gitea.io/gitea/modules/process"
2019
"code.gitea.io/gitea/modules/util"
2120

2221
"github.com/gobwas/glob"
@@ -168,14 +167,11 @@ func generateRepoCommit(e Engine, repo, templateRepo, generateRepo *Repository,
168167
}
169168

170169
repoPath := repo.repoPath(e)
171-
_, stderr, err := process.GetManager().ExecDirEnv(
172-
-1, tmpDir,
173-
fmt.Sprintf("generateRepoCommit(git remote add): %s", repoPath),
174-
env,
175-
git.GitExecutable, "remote", "add", "origin", repoPath,
176-
)
177-
if err != nil {
178-
return fmt.Errorf("git remote add: %v - %s", err, stderr)
170+
if stdout, err := git.NewCommand("remote", "add", "origin", repoPath).
171+
SetDescription(fmt.Sprintf("generateRepoCommit (git remote add): %s to %s", templateRepoPath, tmpDir)).
172+
RunInDirWithEnv(tmpDir, env); err != nil {
173+
log.Error("Unable to add %v as remote origin to temporary repo to %s: stdout %s\nError: %v", repo, tmpDir, stdout, err)
174+
return fmt.Errorf("git remote add: %v", err)
179175
}
180176

181177
return initRepoCommit(tmpDir, repo.Owner)

modules/git/blame.go

+11-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package git
66

77
import (
88
"bufio"
9+
"context"
910
"fmt"
1011
"io"
1112
"os"
@@ -28,6 +29,7 @@ type BlameReader struct {
2829
output io.ReadCloser
2930
scanner *bufio.Scanner
3031
lastSha *string
32+
cancel context.CancelFunc
3133
}
3234

3335
var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})")
@@ -76,7 +78,8 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
7678

7779
// Close BlameReader - don't run NextPart after invoking that
7880
func (r *BlameReader) Close() error {
79-
process.GetManager().Remove(r.pid)
81+
defer process.GetManager().Remove(r.pid)
82+
defer r.cancel()
8083

8184
if err := r.cmd.Wait(); err != nil {
8285
return fmt.Errorf("Wait: %v", err)
@@ -97,20 +100,24 @@ func CreateBlameReader(repoPath, commitID, file string) (*BlameReader, error) {
97100
}
98101

99102
func createBlameReader(dir string, command ...string) (*BlameReader, error) {
100-
cmd := exec.Command(command[0], command[1:]...)
103+
// FIXME: graceful: This should have a timeout
104+
ctx, cancel := context.WithCancel(DefaultContext)
105+
cmd := exec.CommandContext(ctx, command[0], command[1:]...)
101106
cmd.Dir = dir
102107
cmd.Stderr = os.Stderr
103108

104109
stdout, err := cmd.StdoutPipe()
105110
if err != nil {
111+
defer cancel()
106112
return nil, fmt.Errorf("StdoutPipe: %v", err)
107113
}
108114

109115
if err = cmd.Start(); err != nil {
116+
defer cancel()
110117
return nil, fmt.Errorf("Start: %v", err)
111118
}
112119

113-
pid := process.GetManager().Add(fmt.Sprintf("GetBlame [repo_path: %s]", dir), cmd)
120+
pid := process.GetManager().Add(fmt.Sprintf("GetBlame [repo_path: %s]", dir), cancel)
114121

115122
scanner := bufio.NewScanner(stdout)
116123

@@ -120,5 +127,6 @@ func createBlameReader(dir string, command ...string) (*BlameReader, error) {
120127
stdout,
121128
scanner,
122129
nil,
130+
cancel,
123131
}, nil
124132
}

modules/git/command.go

+29-8
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ const DefaultLocale = "C"
3030

3131
// Command represents a command with its subcommands or arguments.
3232
type Command struct {
33-
name string
34-
args []string
33+
name string
34+
args []string
35+
parentContext context.Context
36+
desc string
3537
}
3638

3739
func (c *Command) String() string {
@@ -47,19 +49,34 @@ func NewCommand(args ...string) *Command {
4749
cargs := make([]string, len(GlobalCommandArgs))
4850
copy(cargs, GlobalCommandArgs)
4951
return &Command{
50-
name: GitExecutable,
51-
args: append(cargs, args...),
52+
name: GitExecutable,
53+
args: append(cargs, args...),
54+
parentContext: DefaultContext,
5255
}
5356
}
5457

5558
// NewCommandNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args
5659
func NewCommandNoGlobals(args ...string) *Command {
5760
return &Command{
58-
name: GitExecutable,
59-
args: args,
61+
name: GitExecutable,
62+
args: args,
63+
parentContext: DefaultContext,
6064
}
6165
}
6266

67+
// SetParentContext sets the parent context for this command
68+
func (c *Command) SetParentContext(ctx context.Context) *Command {
69+
c.parentContext = ctx
70+
return c
71+
}
72+
73+
// SetDescription sets the description for this command which be returned on
74+
// c.String()
75+
func (c *Command) SetDescription(desc string) *Command {
76+
c.desc = desc
77+
return c
78+
}
79+
6380
// AddArguments adds new argument(s) to the command.
6481
func (c *Command) AddArguments(args ...string) *Command {
6582
c.args = append(c.args, args...)
@@ -92,7 +109,7 @@ func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.
92109
log("%s: %v", dir, c)
93110
}
94111

95-
ctx, cancel := context.WithTimeout(context.Background(), timeout)
112+
ctx, cancel := context.WithTimeout(c.parentContext, timeout)
96113
defer cancel()
97114

98115
cmd := exec.CommandContext(ctx, c.name, c.args...)
@@ -110,7 +127,11 @@ func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.
110127
return err
111128
}
112129

113-
pid := process.GetManager().Add(fmt.Sprintf("%s %s %s [repo_path: %s]", GitExecutable, c.name, strings.Join(c.args, " "), dir), cmd)
130+
desc := c.desc
131+
if desc == "" {
132+
desc = fmt.Sprintf("%s %s %s [repo_path: %s]", GitExecutable, c.name, strings.Join(c.args, " "), dir)
133+
}
134+
pid := process.GetManager().Add(desc, cancel)
114135
defer process.GetManager().Remove(pid)
115136

116137
if fn != nil {

0 commit comments

Comments
 (0)