Able to set timeout for process monitor

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())

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)
 }

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.ExecTimeout(10*time.Minute,
-	_, stderr, err := process.Exec(fmt.Sprintf("MirrorRepository: %s/%s", userName, repoName),
+		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 {

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
 		}
 

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()
+	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: %v", pid, desc, errKill)
+			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.

templates/VERSION

@@ -1 +1 @@
-0.4.5.0704 Alpha
+0.4.5.0706 Alpha

templates/base/navbar.tmpl

@@ -6,7 +6,7 @@
             <a class="nav-item pull-left{{if .PageIsHelp}} active{{end}}" target="_blank" href="http://gogs.io/docs">Help</a>
             {{if .IsSigned}}
             {{if .HasAccess}}
-            <form class="nav-item pull-left{{if .PageIsNewRepo}} active{{end}}" id="nav-search-form">
+            <!-- <form class="nav-item pull-left{{if .PageIsNewRepo}} active{{end}}" id="nav-search-form">
                 <div class="input-group">
                     <div class="input-group-btn">
                         <button type="button" class="btn btn-sm btn-default dropdown-toggle" data-toggle="dropdown">{{if .Repository}}This Repository{{else}}All Repositories{{end}} <span class="caret"></span></button>
@@ -20,7 +20,7 @@
                     </div>
                     <input type="search" class="form-control input-sm" name="q" placeholder="search code, commits and issues"/>
                 </div>
-            </form>
+            </form> -->
             {{end}}
             <a id="nav-out" class="nav-item navbar-right navbar-btn btn btn-danger" href="/user/logout/"><i class="fa fa-power-off fa-lg"></i></a>
             <a id="nav-avatar" class="nav-item navbar-right{{if .PageIsUserProfile}} active{{end}}" href="{{.SignedUser.HomeLink}}" data-toggle="tooltip" data-placement="bottom" title="{{.SignedUserName}}">