From 60c5339042a605076482320313e8f9498dd72af9 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 30 Nov 2019 08:40:22 -0600 Subject: 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 --- services/gitdiff/gitdiff.go | 29 +++++++++++++++-------- services/mirror/mirror.go | 56 +++++++++++++++++++++++++++++++-------------- services/release/release.go | 11 ++++----- 3 files changed, 64 insertions(+), 32 deletions(-) (limited to 'services') diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 351bfef156..b8efa8a43f 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -8,6 +8,7 @@ package gitdiff import ( "bufio" "bytes" + "context" "fmt" "html" "html/template" @@ -825,9 +826,12 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID return nil, err } + // FIXME: graceful: These commands should likely have a timeout + ctx, cancel := context.WithCancel(git.DefaultContext) + defer cancel() var cmd *exec.Cmd if len(beforeCommitID) == 0 && commit.ParentCount() == 0 { - cmd = exec.Command(git.GitExecutable, "show", afterCommitID) + cmd = exec.CommandContext(ctx, git.GitExecutable, "show", afterCommitID) } else { actualBeforeCommitID := beforeCommitID if len(actualBeforeCommitID) == 0 { @@ -840,7 +844,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID } diffArgs = append(diffArgs, actualBeforeCommitID) diffArgs = append(diffArgs, afterCommitID) - cmd = exec.Command(git.GitExecutable, diffArgs...) + cmd = exec.CommandContext(ctx, git.GitExecutable, diffArgs...) beforeCommitID = actualBeforeCommitID } cmd.Dir = repoPath @@ -855,7 +859,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID return nil, fmt.Errorf("Start: %v", err) } - pid := process.GetManager().Add(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath), cmd) + pid := process.GetManager().Add(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath), cancel) defer process.GetManager().Remove(pid) diff, err := ParsePatch(maxLines, maxLineCharacters, maxFiles, stdout) @@ -908,27 +912,31 @@ func GetRawDiffForFile(repoPath, startCommit, endCommit string, diffType RawDiff if len(file) > 0 { fileArgs = append(fileArgs, "--", file) } + // FIXME: graceful: These commands should have a timeout + ctx, cancel := context.WithCancel(git.DefaultContext) + defer cancel() + var cmd *exec.Cmd switch diffType { case RawDiffNormal: if len(startCommit) != 0 { - cmd = exec.Command(git.GitExecutable, append([]string{"diff", "-M", startCommit, endCommit}, fileArgs...)...) + cmd = exec.CommandContext(ctx, git.GitExecutable, append([]string{"diff", "-M", startCommit, endCommit}, fileArgs...)...) } else if commit.ParentCount() == 0 { - cmd = exec.Command(git.GitExecutable, append([]string{"show", endCommit}, fileArgs...)...) + cmd = exec.CommandContext(ctx, git.GitExecutable, append([]string{"show", endCommit}, fileArgs...)...) } else { c, _ := commit.Parent(0) - cmd = exec.Command(git.GitExecutable, append([]string{"diff", "-M", c.ID.String(), endCommit}, fileArgs...)...) + cmd = exec.CommandContext(ctx, git.GitExecutable, append([]string{"diff", "-M", c.ID.String(), endCommit}, fileArgs...)...) } case RawDiffPatch: if len(startCommit) != 0 { query := fmt.Sprintf("%s...%s", endCommit, startCommit) - cmd = exec.Command(git.GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", "--root", query}, fileArgs...)...) + cmd = exec.CommandContext(ctx, git.GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", "--root", query}, fileArgs...)...) } else if commit.ParentCount() == 0 { - cmd = exec.Command(git.GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", "--root", endCommit}, fileArgs...)...) + cmd = exec.CommandContext(ctx, git.GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", "--root", endCommit}, fileArgs...)...) } else { c, _ := commit.Parent(0) query := fmt.Sprintf("%s...%s", endCommit, c.ID.String()) - cmd = exec.Command(git.GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", query}, fileArgs...)...) + cmd = exec.CommandContext(ctx, git.GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", query}, fileArgs...)...) } default: return fmt.Errorf("invalid diffType: %s", diffType) @@ -939,6 +947,9 @@ func GetRawDiffForFile(repoPath, startCommit, endCommit string, diffType RawDiff cmd.Dir = repoPath cmd.Stdout = writer cmd.Stderr = stderr + pid := process.GetManager().Add(fmt.Sprintf("GetRawDiffForFile: [repo_path: %s]", repoPath), cancel) + defer process.GetManager().Remove(pid) + if err = cmd.Run(); err != nil { return fmt.Errorf("Run: %v - %s", err, stderr) } diff --git a/services/mirror/mirror.go b/services/mirror/mirror.go index d35a205d00..9c52f1723b 100644 --- a/services/mirror/mirror.go +++ b/services/mirror/mirror.go @@ -15,7 +15,6 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" - "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/sync" "code.gitea.io/gitea/modules/timeutil" @@ -172,25 +171,36 @@ func runSync(m *models.Mirror) ([]*mirrorSyncResult, bool) { gitArgs = append(gitArgs, "--prune") } - _, stderr, err := process.GetManager().ExecDir( - timeout, repoPath, fmt.Sprintf("Mirror.runSync: %s", repoPath), - git.GitExecutable, gitArgs...) - if err != nil { + stdoutBuilder := strings.Builder{} + stderrBuilder := strings.Builder{} + if err := git.NewCommand(gitArgs...). + SetDescription(fmt.Sprintf("Mirror.runSync: %s", m.Repo.FullName())). + RunInDirTimeoutPipeline(timeout, repoPath, &stdoutBuilder, &stderrBuilder); err != nil { + stdout := stdoutBuilder.String() + stderr := stderrBuilder.String() // sanitize the output, since it may contain the remote address, which may // contain a password - message, err := sanitizeOutput(stderr, repoPath) + stderrMessage, sanitizeErr := sanitizeOutput(stderr, repoPath) + if sanitizeErr != nil { + log.Error("sanitizeOutput failed on stderr: %v", sanitizeErr) + log.Error("Failed to update mirror repository %v:\nStdout: %s\nStderr: %s\nErr: %v", m.Repo, stdout, stderr, err) + return nil, false + } + stdoutMessage, err := sanitizeOutput(stdout, repoPath) if err != nil { - log.Error("sanitizeOutput: %v", err) + log.Error("sanitizeOutput failed: %v", sanitizeErr) + log.Error("Failed to update mirror repository %v:\nStdout: %s\nStderr: %s\nErr: %v", m.Repo, stdout, stderrMessage, err) return nil, false } - desc := fmt.Sprintf("Failed to update mirror repository '%s': %s", repoPath, message) - log.Error(desc) + + log.Error("Failed to update mirror repository %v:\nStdout: %s\nStderr: %s\nErr: %v", m.Repo, stdoutMessage, stderrMessage, err) + desc := fmt.Sprintf("Failed to update mirror repository '%s': %s", repoPath, stderrMessage) if err = models.CreateRepositoryNotice(desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) } return nil, false } - output := stderr + output := stderrBuilder.String() gitRepo, err := git.OpenRepository(repoPath) if err != nil { @@ -208,18 +218,30 @@ func runSync(m *models.Mirror) ([]*mirrorSyncResult, bool) { } if m.Repo.HasWiki() { - if _, stderr, err := process.GetManager().ExecDir( - timeout, wikiPath, fmt.Sprintf("Mirror.runSync: %s", wikiPath), - git.GitExecutable, "remote", "update", "--prune"); err != nil { + stderrBuilder.Reset() + stdoutBuilder.Reset() + if err := git.NewCommand("remote", "update", "--prune"). + SetDescription(fmt.Sprintf("Mirror.runSync Wiki: %s ", m.Repo.FullName())). + RunInDirTimeoutPipeline(timeout, wikiPath, &stdoutBuilder, &stderrBuilder); err != nil { + stdout := stdoutBuilder.String() + stderr := stderrBuilder.String() // sanitize the output, since it may contain the remote address, which may // contain a password - message, err := sanitizeOutput(stderr, wikiPath) + stderrMessage, sanitizeErr := sanitizeOutput(stderr, repoPath) + if sanitizeErr != nil { + log.Error("sanitizeOutput failed on stderr: %v", sanitizeErr) + log.Error("Failed to update mirror repository wiki %v:\nStdout: %s\nStderr: %s\nErr: %v", m.Repo, stdout, stderr, err) + return nil, false + } + stdoutMessage, err := sanitizeOutput(stdout, repoPath) if err != nil { - log.Error("sanitizeOutput: %v", err) + log.Error("sanitizeOutput failed: %v", sanitizeErr) + log.Error("Failed to update mirror repository wiki %v:\nStdout: %s\nStderr: %s\nErr: %v", m.Repo, stdout, stderrMessage, err) return nil, false } - desc := fmt.Sprintf("Failed to update mirror wiki repository '%s': %s", wikiPath, message) - log.Error(desc) + + log.Error("Failed to update mirror repository wiki %v:\nStdout: %s\nStderr: %s\nErr: %v", m.Repo, stdoutMessage, stderrMessage, err) + desc := fmt.Sprintf("Failed to update mirror repository wiki '%s': %s", wikiPath, stderrMessage) if err = models.CreateRepositoryNotice(desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) } diff --git a/services/release/release.go b/services/release/release.go index 2f70bbb665..fd0c410e7c 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -13,7 +13,6 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" - "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/timeutil" ) @@ -128,11 +127,11 @@ func DeleteReleaseByID(id int64, doer *models.User, delTag bool) error { } if delTag { - _, stderr, err := process.GetManager().ExecDir(-1, repo.RepoPath(), - fmt.Sprintf("DeleteReleaseByID (git tag -d): %d", rel.ID), - git.GitExecutable, "tag", "-d", rel.TagName) - if err != nil && !strings.Contains(stderr, "not found") { - return fmt.Errorf("git tag -d: %v - %s", err, stderr) + if stdout, err := git.NewCommand("tag", "-d", rel.TagName). + SetDescription(fmt.Sprintf("DeleteReleaseByID (git tag -d): %d", rel.ID)). + RunInDir(repo.RepoPath()); err != nil && !strings.Contains(err.Error(), "not found") { + log.Error("DeleteReleaseByID (git tag -d): %d in %v Failed:\nStdout: %s\nError: %v", rel.ID, repo, stdout, err) + return fmt.Errorf("git tag -d: %v", err) } if err := models.DeleteReleaseByID(id); err != nil { -- cgit v1.2.3