From 097c8e05e6791efe3876eedf989e2b69190f7f8f Mon Sep 17 00:00:00 2001 From: Unknown Date: Sun, 6 Jul 2014 17:32:36 -0400 Subject: [PATCH] Able to set timeout for process monitor --- gogs.go | 2 +- models/git_diff.go | 29 +++++++++++++++++----- models/repo.go | 26 +++++++++----------- modules/middleware/repo.go | 2 +- modules/process/manager.go | 49 +++++++++++++++++++++++++++++++++----- templates/VERSION | 2 +- templates/base/navbar.tmpl | 4 ++-- 7 files changed, 82 insertions(+), 32 deletions(-) diff --git a/gogs.go b/gogs.go index 7207baaade..28abaa3ee6 100644 --- a/gogs.go +++ b/gogs.go @@ -17,7 +17,7 @@ import ( "github.com/gogits/gogs/modules/setting" ) -const APP_VER = "0.4.5.0704 Alpha" +const APP_VER = "0.4.5.0706 Alpha" func init() { runtime.GOMAXPROCS(runtime.NumCPU()) diff --git a/models/git_diff.go b/models/git_diff.go index ed114b7504..303d61d5de 100644 --- a/models/git_diff.go +++ b/models/git_diff.go @@ -11,6 +11,7 @@ import ( "os" "os/exec" "strings" + "time" "github.com/gogits/git" @@ -170,10 +171,6 @@ func ParsePatch(pid int64, cmd *exec.Cmd, reader io.Reader) (*Diff, error) { } } - // In case process became zombie. - if err := process.Kill(pid); err != nil { - log.Error("git_diff.ParsePatch(Kill): %v", err) - } return diff, nil } @@ -201,10 +198,30 @@ func GetDiff(repoPath, commitid string) (*Diff, error) { cmd.Stdout = wr cmd.Stdin = os.Stdin cmd.Stderr = os.Stderr + + done := make(chan error) go func() { - cmd.Run() + cmd.Start() + done <- cmd.Wait() wr.Close() }() defer rd.Close() - return ParsePatch(process.Add(fmt.Sprintf("GetDiff(%s)", repoPath), cmd), cmd, rd) + + desc := fmt.Sprintf("GetDiff(%s)", repoPath) + pid := process.Add(desc, cmd) + go func() { + // In case process became zombie. + select { + case <-time.After(5 * time.Minute): + if errKill := process.Kill(pid); errKill != nil { + log.Error("git_diff.ParsePatch(Kill): %v", err) + } + <-done + // return "", ErrExecTimeout.Error(), ErrExecTimeout + case err = <-done: + process.Remove(pid) + } + }() + + return ParsePatch(pid, cmd, rd) } diff --git a/models/repo.go b/models/repo.go index 54177bb6fb..35e4427577 100644 --- a/models/repo.go +++ b/models/repo.go @@ -89,7 +89,7 @@ func NewRepoContext() { // Check if server has basic git setting. stdout, stderr, err := process.Exec("NewRepoContext(get setting)", "git", "config", "--get", "user.name") - if strings.Contains(stderr, "fatal:") { + if err != nil { log.Fatal("repo.NewRepoContext(fail to get git user.name): %s", stderr) } else if err != nil || len(strings.TrimSpace(stdout)) == 0 { if _, stderr, err = process.Exec("NewRepoContext(set email)", "git", "config", "--global", "user.email", "gogitservice@gmail.com"); err != nil { @@ -190,8 +190,8 @@ type Mirror struct { // MirrorRepository creates a mirror repository from source. func MirrorRepository(repoId int64, userName, repoName, repoPath, url string) error { - // TODO: need timeout. - _, stderr, err := process.Exec(fmt.Sprintf("MirrorRepository: %s/%s", userName, repoName), + _, stderr, err := process.ExecTimeout(10*time.Minute, + fmt.Sprintf("MirrorRepository: %s/%s", userName, repoName), "git", "clone", "--mirror", url, repoPath) if err != nil { return errors.New("git clone --mirror: " + stderr) @@ -233,9 +233,8 @@ func MirrorUpdate() { return nil } - // TODO: need timeout. repoPath := filepath.Join(setting.RepoRootPath, m.RepoName+".git") - if _, stderr, err := process.ExecDir( + if _, stderr, err := process.ExecDir(10*time.Minute, repoPath, fmt.Sprintf("MirrorUpdate: %s", repoPath), "git", "remote", "update"); err != nil { return errors.New("git remote update: " + stderr) @@ -272,26 +271,23 @@ func MigrateRepository(u *User, name, desc string, private, mirror bool, url str return repo, UpdateRepository(repo) } - // TODO: need timeout. // Clone from local repository. - _, stderr, err := process.Exec( + _, stderr, err := process.ExecTimeout(10*time.Minute, fmt.Sprintf("MigrateRepository(git clone): %s", repoPath), "git", "clone", repoPath, tmpDir) if err != nil { return repo, errors.New("git clone: " + stderr) } - // TODO: need timeout. // Pull data from source. - if _, stderr, err = process.ExecDir( + if _, stderr, err = process.ExecDir(3*time.Minute, tmpDir, fmt.Sprintf("MigrateRepository(git pull): %s", repoPath), "git", "pull", url); err != nil { return repo, errors.New("git pull: " + stderr) } - // TODO: need timeout. // Push data to local repository. - if _, stderr, err = process.ExecDir( + if _, stderr, err = process.ExecDir(3*time.Minute, tmpDir, fmt.Sprintf("MigrateRepository(git push): %s", repoPath), "git", "push", "origin", "master"); err != nil { return repo, errors.New("git push: " + stderr) @@ -314,20 +310,20 @@ func extractGitBareZip(repoPath string) error { // initRepoCommit temporarily changes with work directory. func initRepoCommit(tmpPath string, sig *git.Signature) (err error) { var stderr string - if _, stderr, err = process.ExecDir( + if _, stderr, err = process.ExecDir(-1, tmpPath, fmt.Sprintf("initRepoCommit(git add): %s", tmpPath), "git", "add", "--all"); err != nil { return errors.New("git add: " + stderr) } - if _, stderr, err = process.ExecDir( + if _, stderr, err = process.ExecDir(-1, tmpPath, fmt.Sprintf("initRepoCommit(git commit): %s", tmpPath), "git", "commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email), "-m", "Init commit"); err != nil { return errors.New("git commit: " + stderr) } - if _, stderr, err = process.ExecDir( + if _, stderr, err = process.ExecDir(-1, tmpPath, fmt.Sprintf("initRepoCommit(git push): %s", tmpPath), "git", "push", "origin", "master"); err != nil { return errors.New("git push: " + stderr) @@ -583,7 +579,7 @@ func CreateRepository(u *User, name, desc, lang, license string, private, mirror return nil, err } - _, stderr, err := process.ExecDir( + _, stderr, err := process.ExecDir(-1, repoPath, fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath), "git", "update-server-info") if err != nil { diff --git a/modules/middleware/repo.go b/modules/middleware/repo.go index 8aa4a6a8a6..cf8ce412e5 100644 --- a/modules/middleware/repo.go +++ b/modules/middleware/repo.go @@ -82,7 +82,7 @@ func RepoAssignment(redirect bool, args ...bool) martini.Handler { ctx.Repo.Owner = user // Organization owner team members are true owners as well. - if ctx.Repo.Owner.IsOrganization() && ctx.Repo.Owner.IsOrgOwner(ctx.User.Id) { + if ctx.IsSigned && ctx.Repo.Owner.IsOrganization() && ctx.Repo.Owner.IsOrgOwner(ctx.User.Id) { ctx.Repo.IsTrueOwner = true } diff --git a/modules/process/manager.go b/modules/process/manager.go index 173b2aa4ee..ce8ab7b4b2 100644 --- a/modules/process/manager.go +++ b/modules/process/manager.go @@ -6,6 +6,7 @@ package process import ( "bytes" + "errors" "fmt" "os/exec" "time" @@ -13,6 +14,16 @@ import ( "github.com/gogits/gogs/modules/log" ) +var ( + ErrExecTimeout = errors.New("Process execution timeout") +) + +// Common timeout. +var ( + // NOTE: could be custom in config file for default. + DEFAULT = 60 * time.Second +) + // Process represents a working process inherit from Gogs. type Process struct { Pid int64 // Process ID, not system one. @@ -40,7 +51,12 @@ func Add(desc string, cmd *exec.Cmd) int64 { return pid } -func ExecDir(dir, desc, cmdName string, args ...string) (string, string, error) { +// Exec starts executing a command in given path, it records its process and timeout. +func ExecDir(timeout time.Duration, dir, desc, cmdName string, args ...string) (string, string, error) { + if timeout == -1 { + timeout = DEFAULT + } + bufOut := new(bytes.Buffer) bufErr := new(bytes.Buffer) @@ -48,18 +64,39 @@ func ExecDir(dir, desc, cmdName string, args ...string) (string, string, error) cmd.Dir = dir cmd.Stdout = bufOut cmd.Stderr = bufErr + if err := cmd.Start(); err != nil { + return "", err.Error(), err + } pid := Add(desc, cmd) - err := cmd.Run() - if errKill := Kill(pid); errKill != nil { - log.Error("Exec: %v", pid, desc, errKill) + done := make(chan error) + go func() { + done <- cmd.Wait() + }() + + var err error + select { + case <-time.After(timeout): + if errKill := Kill(pid); errKill != nil { + log.Error("Exec(%d:%s): %v", pid, desc, errKill) + } + <-done + return "", ErrExecTimeout.Error(), ErrExecTimeout + case err = <-done: } + + Remove(pid) return bufOut.String(), bufErr.String(), err } -// Exec starts executing a command and record its process. +// Exec starts executing a command, it records its process and timeout. +func ExecTimeout(timeout time.Duration, desc, cmdName string, args ...string) (string, string, error) { + return ExecDir(timeout, "", desc, cmdName, args...) +} + +// Exec starts executing a command, it records its process and has default timeout. func Exec(desc, cmdName string, args ...string) (string, string, error) { - return ExecDir("", desc, cmdName, args...) + return ExecDir(-1, "", desc, cmdName, args...) } // Remove removes a process from list. diff --git a/templates/VERSION b/templates/VERSION index 6237548bbc..9821ab4ed5 100644 --- a/templates/VERSION +++ b/templates/VERSION @@ -1 +1 @@ -0.4.5.0704 Alpha \ No newline at end of file +0.4.5.0706 Alpha \ No newline at end of file diff --git a/templates/base/navbar.tmpl b/templates/base/navbar.tmpl index 29a2b75cc1..dcb9a644c1 100644 --- a/templates/base/navbar.tmpl +++ b/templates/base/navbar.tmpl @@ -6,7 +6,7 @@ Help {{if .IsSigned}} {{if .HasAccess}} -