Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix possible deadlock/panic when detaching a process

Remove fork() call which often caused deadlock on linux and
panic on macosx.  We now reap any process we start via Wait()
in a go routine for those who do not self detach.

Change-Id: Ifa526561f8c385bc8d000310e0445ba87bf56bfe
  • Loading branch information...
commit 8d7c5a528a1d7b61075ce69cdc649300f5137410 1 parent 9244ee3
@dougm dougm authored
Showing with 28 additions and 93 deletions.
  1. +8 −85 process.go
  2. +12 −7 process_test.go
  3. +8 −1 test/helper/helper.go
View
93 process.go
@@ -12,7 +12,6 @@ import (
"os"
"os/exec"
"os/user"
- "runtime"
"strconv"
"strings"
"syscall"
@@ -109,80 +108,21 @@ func (p *Process) Spawn(program string) (*exec.Cmd, error) {
// manage its own Pidfile.
// Otherwise; we will detach the process and manage its Pidfile.
func (p *Process) StartProcess() (int, error) {
- if p.Detached {
- cmd, err := p.Spawn(p.Start)
- if err != nil {
- return 0, err
- }
-
- err = cmd.Wait()
-
- return cmd.Process.Pid, err
- }
-
- pr, pw, err := os.Pipe()
+ cmd, err := p.Spawn(p.Start)
if err != nil {
return 0, err
}
- defer pr.Close()
- defer pw.Close()
-
- ret, err := fork()
- if err != nil {
- return 0, err
- }
-
- if ret == 0 { // child
- pr.Close()
-
- status := 0
-
- cmd, err := p.Spawn(p.Start)
- if err == nil {
- // write pid to parent
- err = binary.Write(pw, byteOrder, int32(cmd.Process.Pid))
-
- cmd.Process.Release()
- }
-
- if err != nil {
- // propagate errno to parent via exit status
- if perr, ok := err.(*os.PathError); ok {
- status = int(perr.Err.(syscall.Errno))
- os.Exit(status)
- }
- os.Exit(1)
- }
-
- os.Exit(0)
- } else { // parent
- pw.Close()
-
- var status syscall.WaitStatus
-
- _, err := syscall.Wait4(int(ret), &status, 0, nil)
+ pid := cmd.Process.Pid
- if err != nil {
- return 0, err
- }
-
- if status.ExitStatus() != 0 {
- return -1, syscall.Errno(status.ExitStatus())
- }
-
- var pid int32
- // read pid from child
- err = binary.Read(pr, byteOrder, &pid)
-
- if err == nil {
- err = p.SavePid(int(pid))
- }
-
- return int(pid), err
+ if p.Detached {
+ err = cmd.Wait()
+ } else {
+ err = p.SavePid(pid)
+ go cmd.Wait()
}
- panic("not reached") // shutup compiler
+ return pid, err
}
// Stop a process:
@@ -320,23 +260,6 @@ func (p *Process) lookupCredentials(credential *syscall.Credential) error {
return nil
}
-// go does not have a fork() wrapper w/o exec
-func fork() (int, error) {
- darwin := runtime.GOOS == "darwin"
-
- ret, ret2, errno := syscall.RawSyscall(syscall.SYS_FORK, 0, 0, 0)
- if errno != 0 {
- return 0, errno
- }
-
- // see syscall/exec_bsd.go
- if darwin && ret2 == 1 {
- ret = 0
- }
-
- return int(ret), nil
-}
-
// until we have user.LookupGroupId: http://codereview.appspot.com/4589049
func LookupGroupId(group string) (int, error) {
const (
View
19 process_test.go
@@ -11,7 +11,6 @@ import (
"path/filepath"
"sort"
"strings"
- "syscall"
"testing"
"time"
)
@@ -32,13 +31,22 @@ func processInfo(p *Process) *helper.ProcessInfo {
// these assertions apply to any daemon process
func assertProcessInfo(t *testing.T, process *Process, info *helper.ProcessInfo) {
selfInfo := helper.CurrentProcessInfo()
+ var ppid int
- assert.Equal(t, 1, info.Ppid) // parent pid is init (1)
+ if process.Detached {
+ ppid = 1
+ } else {
+ ppid = selfInfo.Pid
+ }
+
+ assert.Equal(t, ppid, info.Ppid) // parent pid
assert.NotEqual(t, selfInfo.Pgrp, info.Pgrp) // process group will change
assert.NotEqual(t, selfInfo.Sid, info.Sid) // session id will change
+ assert.Equal(t, false, info.HasTty) // no controling terminal
+
sort.Strings(selfInfo.Env)
sort.Strings(info.Env)
assert.NotEqual(t, selfInfo.Env, info.Env) // sanitized env will differ
@@ -271,11 +279,8 @@ func TestFailExe(t *testing.T) {
}
_, err = process.StartProcess()
- if process.Detached {
- assert.NotEqual(t, nil, err)
- } else {
- assert.Equal(t, syscall.EPERM, err)
- }
+ assert.NotEqual(t, nil, err)
+
pause()
assert.Equal(t, false, process.IsRunning())
View
9 test/helper/helper.go
@@ -32,16 +32,23 @@ type ProcessInfo struct {
Args []string
Env []string
Restarts int
+ HasTty bool
}
var TestProcess, goprocess string
func CurrentProcessInfo() *ProcessInfo {
+ var hasTty bool
cwd, _ := os.Getwd()
grp, _ := os.Getgroups()
// no syscall.Getsid() wrapper on Linux?
sid, _, _ := syscall.RawSyscall(syscall.SYS_GETSID, 0, 0, 0)
+ if fh, err := os.Open("/dev/tty"); err == nil {
+ hasTty = true
+ fh.Close()
+ }
+
return &ProcessInfo{
Ppid: os.Getppid(),
Pid: os.Getpid(),
@@ -55,6 +62,7 @@ func CurrentProcessInfo() *ProcessInfo {
Groups: grp,
Args: os.Args,
Env: os.Environ(),
+ HasTty: hasTty,
}
}
@@ -193,7 +201,6 @@ func mkcmd(args []string, action string) []string {
}
func NewTestProcess(name string, flags []string, detached bool) *Process {
- detached = true // XXX remove when detaching works properly
// using '/tmp' rather than os.TempDir, otherwise 'sudo -E go test'
// will fail on darwin, since only the user that started the process
// has rx perms
Please sign in to comment.
Something went wrong with that request. Please try again.