From 4c61bb7cb7176978be727a3f9d4f277dd81715f5 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 07:51:07 +0100 Subject: [PATCH 01/25] Refactor colored writing into WriteLine() function This serves two purposes. It protects with a Mutex where it was missing before, and makes it so that there is only one place to change code for the line writing, which will be a later pull request. --- outlet.go | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/outlet.go b/outlet.go index ffc42a8..a98613a 100644 --- a/outlet.go +++ b/outlet.go @@ -40,20 +40,9 @@ func NewOutletFactory() (of *OutletFactory) { } func (o *Outlet) Write(b []byte) (num int, err error) { - mx.Lock() - defer mx.Unlock() scanner := bufio.NewScanner(bytes.NewReader(b)) for scanner.Scan() { - formatter := fmt.Sprintf("%%-%ds | ", o.Factory.Padding) - ct.ChangeColor(o.Color, true, ct.None, false) - fmt.Printf(formatter, o.Name) - if o.IsError { - ct.ChangeColor(ct.Red, true, ct.None, true) - } else { - ct.ResetColor() - } - fmt.Println(scanner.Text()) - ct.ResetColor() + o.Factory.WriteLine(o.Name, scanner.Text(), ct.White, ct.None, o.IsError) } num = len(b) return @@ -69,15 +58,27 @@ func (of *OutletFactory) CreateOutlet(name string, index int, isError bool) *Out } func (of *OutletFactory) SystemOutput(str string) { - ct.ChangeColor(ct.White, true, ct.None, false) - formatter := fmt.Sprintf("%%-%ds | ", of.Padding) - fmt.Printf(formatter, "forego") - ct.ResetColor() - fmt.Println(str) - ct.ResetColor() + of.WriteLine("forego", str, ct.White, ct.None, false) } func (of *OutletFactory) ErrorOutput(str string) { fmt.Printf("ERROR: %s\n", str) os.Exit(1) } + +// Write out a single coloured line +func (of *OutletFactory) WriteLine(left, right string, leftC, rightC ct.Color, isError bool) { + mx.Lock() + defer mx.Unlock() + + ct.ChangeColor(leftC, true, ct.None, false) + formatter := fmt.Sprintf("%%-%ds | ", of.Padding) + fmt.Printf(formatter, left) + + if isError { + ct.ChangeColor(ct.Red, true, ct.None, true) + } else { + ct.ResetColor() + } + fmt.Println(right) +} From 826329f42605fa53b0cece44eae410076f74992f Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 07:54:07 +0100 Subject: [PATCH 02/25] Move mutex onto OutletFactory It's bad form for the mutex to live in the global namespace when it doesn't need to. --- outlet.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/outlet.go b/outlet.go index a98613a..df2ea45 100644 --- a/outlet.go +++ b/outlet.go @@ -13,6 +13,8 @@ import ( type OutletFactory struct { Outlets map[string]*Outlet Padding int + + sync.Mutex } type Outlet struct { @@ -22,8 +24,6 @@ type Outlet struct { Factory *OutletFactory } -var mx sync.Mutex - var colors = []ct.Color{ ct.Cyan, ct.Yellow, @@ -68,8 +68,8 @@ func (of *OutletFactory) ErrorOutput(str string) { // Write out a single coloured line func (of *OutletFactory) WriteLine(left, right string, leftC, rightC ct.Color, isError bool) { - mx.Lock() - defer mx.Unlock() + of.Lock() + defer of.Unlock() ct.ChangeColor(leftC, true, ct.None, false) formatter := fmt.Sprintf("%%-%ds | ", of.Padding) From 9fd558a9cc0ce64234ab17e7fab353b0fdd4b9d9 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 07:59:38 +0100 Subject: [PATCH 03/25] Simplify procName calculation --- start.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/start.go b/start.go index e20d8f0..e0e5967 100644 --- a/start.go +++ b/start.go @@ -79,9 +79,7 @@ func startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFacto wg.Add(1) port := flagPort + (idx * 100) ps := NewProcess(proc.Command, env) - procName := strings.Join([]string{ - proc.Name, - strconv.FormatInt(int64(procNum+1), 10)}, ".") + procName := fmt.Sprint(proc.Name, ".", procNum+1) processes[procName] = ps ps.Env["PORT"] = strconv.Itoa(port) ps.Root = filepath.Dir(flagProcfile) @@ -89,6 +87,7 @@ func startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFacto ps.Stdout = of.CreateOutlet(procName, idx, false) ps.Stderr = of.CreateOutlet(procName, idx, true) ps.Start() + of.SystemOutput(fmt.Sprintf("starting %s on port %d", procName, port)) go func(proc ProcfileEntry, ps *Process) { ps.Wait() From eb103b584306b7ae4725240e79e0f4d3c3896087 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 08:00:33 +0100 Subject: [PATCH 04/25] Use defer to mark goroutine completed Also, move the wg.Add() next to the point where it matters. --- start.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/start.go b/start.go index e0e5967..c7df425 100644 --- a/start.go +++ b/start.go @@ -76,7 +76,6 @@ func parseConcurrency(value string) (map[string]int, error) { func startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFactory) { shutdown_mutex.Lock() - wg.Add(1) port := flagPort + (idx * 100) ps := NewProcess(proc.Command, env) procName := fmt.Sprint(proc.Name, ".", procNum+1) @@ -89,17 +88,18 @@ func startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFacto ps.Start() of.SystemOutput(fmt.Sprintf("starting %s on port %d", procName, port)) + + wg.Add(1) go func(proc ProcfileEntry, ps *Process) { + defer wg.Done() ps.Wait() if flagRestart && !shutdownNow { delete(processes, proc.Name) startProcess(idx, procNum, proc, env, of) - wg.Done() return } - wg.Done() delete(processes, proc.Name) ShutdownProcesses(of) From c3001f494138575ecf730ef571949510d3c099d4 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 08:01:49 +0100 Subject: [PATCH 05/25] Use defer to unlock mutex This is safer in general, in case a function exits for any reason, including due to a panic (which may be recovered later). --- start.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/start.go b/start.go index c7df425..6c52f78 100644 --- a/start.go +++ b/start.go @@ -76,7 +76,10 @@ func parseConcurrency(value string) (map[string]int, error) { func startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFactory) { shutdown_mutex.Lock() + defer shutdown_mutex.Unlock() + port := flagPort + (idx * 100) + ps := NewProcess(proc.Command, env) procName := fmt.Sprint(proc.Name, ".", procNum+1) processes[procName] = ps @@ -102,9 +105,7 @@ func startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFacto delete(processes, proc.Name) ShutdownProcesses(of) - }(proc, ps) - shutdown_mutex.Unlock() } func runStart(cmd *Command, args []string) { From 9c2bf670ca7e70fb304c8ce5121ca25880947db8 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 08:37:21 +0100 Subject: [PATCH 06/25] Refactor interrupt monitoring and shutdown This makes it so that the same goroutine responsible for starting processes shuts them down. This is a stepping stone to a larger refactor whose goal is to eliminate race conditions. --- start.go | 62 ++++++++++++++++++++++++++++++++++-------------------- unix.go | 2 -- windows.go | 2 -- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/start.go b/start.go index 6c52f78..28bed0c 100644 --- a/start.go +++ b/start.go @@ -20,8 +20,6 @@ var flagRestart bool var shutdownNow bool var processes = map[string]*Process{} -var shutdown_mutex = new(sync.Mutex) -var wg sync.WaitGroup var cmdStart = &Command{ Run: runStart, @@ -74,10 +72,13 @@ func parseConcurrency(value string) (map[string]int, error) { return concurrency, nil } -func startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFactory) { - shutdown_mutex.Lock() - defer shutdown_mutex.Unlock() +type Forego struct { + shutdown sync.Once + teardown chan struct{} // barrier: closed when shutting down + wg sync.WaitGroup +} +func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFactory) { port := flagPort + (idx * 100) ps := NewProcess(proc.Command, env) @@ -92,22 +93,41 @@ func startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFacto of.SystemOutput(fmt.Sprintf("starting %s on port %d", procName, port)) - wg.Add(1) + f.wg.Add(1) go func(proc ProcfileEntry, ps *Process) { - defer wg.Done() + defer f.wg.Done() + ps.Wait() if flagRestart && !shutdownNow { delete(processes, proc.Name) - startProcess(idx, procNum, proc, env, of) + f.startProcess(idx, procNum, proc, env, of) return } delete(processes, proc.Name) - ShutdownProcesses(of) + f.SignalShutdown() }(proc, ps) } +func (f *Forego) SignalShutdown() { + f.shutdown.Do(func() { + close(f.teardown) + }) +} + +func (f *Forego) monitorInterrupt() { + handler := make(chan os.Signal, 1) + signal.Notify(handler, os.Interrupt) + for sig := range handler { + switch sig { + case os.Interrupt: + fmt.Println(" | ctrl-c detected") + f.SignalShutdown() + } + } +} + func runStart(cmd *Command, args []string) { root := filepath.Dir(flagProcfile) @@ -127,19 +147,11 @@ func runStart(cmd *Command, args []string) { of := NewOutletFactory() of.Padding = pf.LongestProcessName() - handler := make(chan os.Signal, 1) - signal.Notify(handler, os.Interrupt) + f := &Forego{ + teardown: make(chan struct{}), + } - go func() { - for sig := range handler { - switch sig { - case os.Interrupt: - shutdownNow = true - fmt.Println(" | ctrl-c detected") - go func() { ShutdownProcesses(of) }() - } - } - }() + go f.monitorInterrupt() var singleton string = "" if len(args) > 0 { @@ -156,10 +168,14 @@ func runStart(cmd *Command, args []string) { } for i := 0; i < numProcs; i++ { if (singleton == "") || (singleton == proc.Name) { - startProcess(idx, i, proc, env, of) + f.startProcess(idx, i, proc, env, of) } } } - wg.Wait() + <-f.teardown + of.SystemOutput("shutting down") + ShutdownProcesses(of) + + f.wg.Wait() } diff --git a/unix.go b/unix.go index 55c8db6..73ef090 100644 --- a/unix.go +++ b/unix.go @@ -34,8 +34,6 @@ func (p *Process) Signal(signal syscall.Signal) { } func ShutdownProcesses(of *OutletFactory) { - shutdown_mutex.Lock() - of.SystemOutput("shutting down") for name, ps := range processes { of.SystemOutput(fmt.Sprintf("sending SIGTERM to %s", name)) ps.Signal(syscall.SIGTERM) diff --git a/windows.go b/windows.go index ee14e3b..862f95f 100644 --- a/windows.go +++ b/windows.go @@ -26,8 +26,6 @@ func (p *Process) Signal(signal syscall.Signal) { } func ShutdownProcesses(of *OutletFactory) { - shutdown_mutex.Lock() - of.SystemOutput("shutting down") for name, ps := range processes { of.SystemOutput(fmt.Sprintf("terminating %s", name)) ps.cmd.Process.Signal(os.Kill) From ca11ca63881d5f8ce8ce3d3aced086fb54362b99 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 08:44:06 +0100 Subject: [PATCH 07/25] Refactor ShutdownProcess to smaller platform-specific part --- start.go | 6 ++++++ unix.go | 14 +++++--------- windows.go | 9 +++------ 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/start.go b/start.go index 28bed0c..992726f 100644 --- a/start.go +++ b/start.go @@ -179,3 +179,9 @@ func runStart(cmd *Command, args []string) { f.wg.Wait() } + +func ShutdownProcesses(of *OutletFactory) { + for name, p := range processes { + ShutdownProcess(of, p, name) + } +} diff --git a/unix.go b/unix.go index 73ef090..e49d609 100644 --- a/unix.go +++ b/unix.go @@ -33,16 +33,12 @@ func (p *Process) Signal(signal syscall.Signal) { } } -func ShutdownProcesses(of *OutletFactory) { - for name, ps := range processes { - of.SystemOutput(fmt.Sprintf("sending SIGTERM to %s", name)) - ps.Signal(syscall.SIGTERM) - } +func ShutdownProcess(of *OutletFactory, ps *Process, name string) { + of.SystemOutput(fmt.Sprintf("sending SIGTERM to %s", name)) + ps.Signal(syscall.SIGTERM) go func() { time.Sleep(shutdownGraceTime) - for name, ps := range processes { - of.SystemOutput(fmt.Sprintf("sending SIGKILL to %s", name)) - ps.Signal(syscall.SIGKILL) - } + of.SystemOutput(fmt.Sprintf("sending SIGKILL to %s", name)) + ps.Signal(syscall.SIGKILL) }() } diff --git a/windows.go b/windows.go index 862f95f..373647c 100644 --- a/windows.go +++ b/windows.go @@ -25,10 +25,7 @@ func (p *Process) Signal(signal syscall.Signal) { group.Signal(signal) } -func ShutdownProcesses(of *OutletFactory) { - for name, ps := range processes { - of.SystemOutput(fmt.Sprintf("terminating %s", name)) - ps.cmd.Process.Signal(os.Kill) - } - os.Exit(1) +func ShutdownProcess(of *OutletFactory, ps *Process, name string) { + of.SystemOutput(fmt.Sprintf("terminating %s", name)) + ps.cmd.Process.Signal(os.Kill) } From a5074c5c37e147532d6591bbfd55aabc00051f62 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 09:28:41 +0100 Subject: [PATCH 08/25] Refactor process shutdown This is a large change which makes startProcess() responsible for shutting its own process down, rather than having a global map. This makes it much easier to reason about who owns what and get the synchronization right. The main immediate bugfix is that now we only attempt to send SIGKILL to processes which haven't yet shut down, making it clearer which processes are responsible for a delayed shutdown. It introduces a `teardown` channel which is closed to signal to all process-monitoring goroutines that they should clean up. In addition, the platform-specific code has been thinned. Some of the platform logic moved into the process-monitoring code because it needed to for now after the introduction of the <-finished barrier. --- start.go | 108 ++++++++++++++++++++++++++++++++--------------------- unix.go | 17 ++++----- windows.go | 11 ++++-- 3 files changed, 82 insertions(+), 54 deletions(-) diff --git a/start.go b/start.go index 992726f..b882510 100644 --- a/start.go +++ b/start.go @@ -17,9 +17,6 @@ const shutdownGraceTime = 3 * time.Second var flagPort int var flagConcurrency string var flagRestart bool -var shutdownNow bool - -var processes = map[string]*Process{} var cmdStart = &Command{ Run: runStart, @@ -73,9 +70,33 @@ func parseConcurrency(value string) (map[string]int, error) { } type Forego struct { - shutdown sync.Once + shutdown sync.Once // Closes teardown exactly once teardown chan struct{} // barrier: closed when shutting down - wg sync.WaitGroup + + wg sync.WaitGroup +} + +func (f *Forego) SignalShutdown() { + f.shutdown.Do(func() { close(f.teardown) }) +} + +func (f *Forego) monitorInterrupt() { + handler := make(chan os.Signal, 1) + signal.Notify(handler, os.Interrupt) + + first := true + + for sig := range handler { + switch sig { + case os.Interrupt: + fmt.Println(" | ctrl-c detected") + + if !first { + + } + f.SignalShutdown() + } + } } func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFactory) { @@ -83,49 +104,59 @@ func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of ps := NewProcess(proc.Command, env) procName := fmt.Sprint(proc.Name, ".", procNum+1) - processes[procName] = ps ps.Env["PORT"] = strconv.Itoa(port) ps.Root = filepath.Dir(flagProcfile) ps.Stdin = nil ps.Stdout = of.CreateOutlet(procName, idx, false) ps.Stderr = of.CreateOutlet(procName, idx, true) - ps.Start() of.SystemOutput(fmt.Sprintf("starting %s on port %d", procName, port)) + finished := make(chan struct{}) // closed on process exit + + ps.Start() + go func() { + defer close(finished) + ps.Wait() + }() + f.wg.Add(1) - go func(proc ProcfileEntry, ps *Process) { + go func() { defer f.wg.Done() - ps.Wait() + // Prevent goroutine from exiting before process has finished. + defer func() { <-finished }() - if flagRestart && !shutdownNow { - delete(processes, proc.Name) - f.startProcess(idx, procNum, proc, env, of) - return + select { + case <-finished: + if flagRestart { + f.startProcess(idx, procNum, proc, env, of) + return + } else { + f.SignalShutdown() + } + + case <-f.teardown: + // Forego tearing down + + if !osHaveSigTerm { + of.SystemOutput(fmt.Sprintf("Killing %s", procName)) + ps.cmd.Process.Kill() + return + } + + of.SystemOutput(fmt.Sprintf("sending SIGTERM to %s", procName)) + ps.SendSigTerm() + + // Give the process a chance to exit, otherwise kill it. + select { + case <-time.After(shutdownGraceTime): + of.SystemOutput(fmt.Sprintf("Killing %s", procName)) + ps.SendSigKill() + case <-finished: + } } - - delete(processes, proc.Name) - f.SignalShutdown() - }(proc, ps) -} - -func (f *Forego) SignalShutdown() { - f.shutdown.Do(func() { - close(f.teardown) - }) -} - -func (f *Forego) monitorInterrupt() { - handler := make(chan os.Signal, 1) - signal.Notify(handler, os.Interrupt) - for sig := range handler { - switch sig { - case os.Interrupt: - fmt.Println(" | ctrl-c detected") - f.SignalShutdown() - } - } + }() } func runStart(cmd *Command, args []string) { @@ -175,13 +206,6 @@ func runStart(cmd *Command, args []string) { <-f.teardown of.SystemOutput("shutting down") - ShutdownProcesses(of) f.wg.Wait() } - -func ShutdownProcesses(of *OutletFactory) { - for name, p := range processes { - ShutdownProcess(of, p, name) - } -} diff --git a/unix.go b/unix.go index e49d609..0e0151c 100644 --- a/unix.go +++ b/unix.go @@ -8,9 +8,10 @@ import ( "os/exec" "path/filepath" "syscall" - "time" ) +const osHaveSigTerm = true + func (p *Process) Start() { command := []string{"/bin/bash", p.shellArgument(), fmt.Sprintf("source \"%s\" 2>/dev/null; %s", filepath.Join(p.Root, ".profile"), p.Command)} p.cmd = exec.Command(command[0], command[1:]...) @@ -33,12 +34,10 @@ func (p *Process) Signal(signal syscall.Signal) { } } -func ShutdownProcess(of *OutletFactory, ps *Process, name string) { - of.SystemOutput(fmt.Sprintf("sending SIGTERM to %s", name)) - ps.Signal(syscall.SIGTERM) - go func() { - time.Sleep(shutdownGraceTime) - of.SystemOutput(fmt.Sprintf("sending SIGKILL to %s", name)) - ps.Signal(syscall.SIGKILL) - }() +func (p *Process) SendSigTerm() { + p.Signal(syscall.SIGTERM) +} + +func (p *Process) SendSigKill() { + p.Signal(syscall.SIGKILL) } diff --git a/windows.go b/windows.go index 373647c..d55b67b 100644 --- a/windows.go +++ b/windows.go @@ -9,6 +9,8 @@ import ( "syscall" ) +const osHaveSigTerm = false + func (p *Process) Start() { command := []string{"cmd", "/C", p.Command} p.cmd = exec.Command(command[0], command[1:]...) @@ -25,7 +27,10 @@ func (p *Process) Signal(signal syscall.Signal) { group.Signal(signal) } -func ShutdownProcess(of *OutletFactory, ps *Process, name string) { - of.SystemOutput(fmt.Sprintf("terminating %s", name)) - ps.cmd.Process.Signal(os.Kill) +func (p *Process) SendSigTerm() { + panic("SendSigTerm() not implemented on this platform") +} + +func (p *Process) SendSigKill() { + p.Signal(syscall.SIGKILL) } From 1bc3decf0c3365dfaca70d8e3d3b235e300becf9 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 09:35:15 +0100 Subject: [PATCH 09/25] Implement teardownNow with Kill on 2nd CTRL-C --- start.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/start.go b/start.go index b882510..11fd8e9 100644 --- a/start.go +++ b/start.go @@ -70,8 +70,9 @@ func parseConcurrency(value string) (map[string]int, error) { } type Forego struct { - shutdown sync.Once // Closes teardown exactly once - teardown chan struct{} // barrier: closed when shutting down + shutdown sync.Once // Closes teardown exactly once + teardown chan struct{} // barrier: closed when shutting down + teardownNow chan struct{} // barrier: second CTRL-C. More urgent. wg sync.WaitGroup } @@ -85,6 +86,7 @@ func (f *Forego) monitorInterrupt() { signal.Notify(handler, os.Interrupt) first := true + var once sync.Once for sig := range handler { switch sig { @@ -92,9 +94,10 @@ func (f *Forego) monitorInterrupt() { fmt.Println(" | ctrl-c detected") if !first { - + once.Do(func() { close(f.teardownNow) }) } f.SignalShutdown() + first = false } } } @@ -153,6 +156,9 @@ func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of case <-time.After(shutdownGraceTime): of.SystemOutput(fmt.Sprintf("Killing %s", procName)) ps.SendSigKill() + case <-f.teardownNow: + of.SystemOutput(fmt.Sprintf("Killing %s", procName)) + ps.SendSigKill() case <-finished: } } @@ -179,7 +185,8 @@ func runStart(cmd *Command, args []string) { of.Padding = pf.LongestProcessName() f := &Forego{ - teardown: make(chan struct{}), + teardown: make(chan struct{}), + teardownNow: make(chan struct{}), } go f.monitorInterrupt() From f2170857e684fb7236b49d7794b3660e3806ce5a Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 09:41:37 +0100 Subject: [PATCH 10/25] Move 'shutting down' msg to before close(teardown) --- start.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/start.go b/start.go index 11fd8e9..a40dc75 100644 --- a/start.go +++ b/start.go @@ -70,15 +70,19 @@ func parseConcurrency(value string) (map[string]int, error) { } type Forego struct { - shutdown sync.Once // Closes teardown exactly once - teardown chan struct{} // barrier: closed when shutting down - teardownNow chan struct{} // barrier: second CTRL-C. More urgent. + outletFactory *OutletFactory + shutdown sync.Once // Closes teardown exactly once + teardown chan struct{} // barrier: closed when shutting down + teardownNow chan struct{} // barrier: second CTRL-C. More urgent. wg sync.WaitGroup } func (f *Forego) SignalShutdown() { - f.shutdown.Do(func() { close(f.teardown) }) + f.shutdown.Do(func() { + f.outletFactory.SystemOutput("shutting down") + close(f.teardown) + }) } func (f *Forego) monitorInterrupt() { @@ -185,6 +189,8 @@ func runStart(cmd *Command, args []string) { of.Padding = pf.LongestProcessName() f := &Forego{ + outletFactory: of, + teardown: make(chan struct{}), teardownNow: make(chan struct{}), } @@ -212,7 +218,6 @@ func runStart(cmd *Command, args []string) { } <-f.teardown - of.SystemOutput("shutting down") f.wg.Wait() } From 6f180b7482c36592770cc06acac1877fbb11d6a4 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 17:20:45 +0100 Subject: [PATCH 11/25] Move envAsArray from Process onto Env --- env.go | 11 +++++++++++ process.go | 11 ----------- unix.go | 2 +- windows.go | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/env.go b/env.go index 36aa500..c162fd1 100644 --- a/env.go +++ b/env.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "github.com/subosito/gotenv" "os" "regexp" @@ -25,3 +26,13 @@ func ReadEnv(filename string) (Env, error) { } return env, nil } + +func (e *Env) asArray() (env []string) { + for _, pair := range os.Environ() { + env = append(env, pair) + } + for name, val := range *e { + env = append(env, fmt.Sprintf("%s=%s", name, val)) + } + return +} diff --git a/process.go b/process.go index e107dca..30fe207 100644 --- a/process.go +++ b/process.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "io" "os" "os/exec" @@ -50,13 +49,3 @@ func (p *Process) shellArgument() string { return "-c" } } - -func (p *Process) envAsArray() (env []string) { - for _, pair := range os.Environ() { - env = append(env, pair) - } - for name, val := range p.Env { - env = append(env, fmt.Sprintf("%s=%s", name, val)) - } - return -} diff --git a/unix.go b/unix.go index 0e0151c..14cdbdf 100644 --- a/unix.go +++ b/unix.go @@ -16,7 +16,7 @@ func (p *Process) Start() { command := []string{"/bin/bash", p.shellArgument(), fmt.Sprintf("source \"%s\" 2>/dev/null; %s", filepath.Join(p.Root, ".profile"), p.Command)} p.cmd = exec.Command(command[0], command[1:]...) p.cmd.Dir = p.Root - p.cmd.Env = p.envAsArray() + p.cmd.Env = p.Env.asArray() p.cmd.Stdin = p.Stdin p.cmd.Stdout = p.Stdout p.cmd.Stderr = p.Stderr diff --git a/windows.go b/windows.go index d55b67b..a45a609 100644 --- a/windows.go +++ b/windows.go @@ -15,7 +15,7 @@ func (p *Process) Start() { command := []string{"cmd", "/C", p.Command} p.cmd = exec.Command(command[0], command[1:]...) p.cmd.Dir = p.Root - p.cmd.Env = p.envAsArray() + p.cmd.Env = p.Env.asArray() p.cmd.Stdin = p.Stdin p.cmd.Stdout = p.Stdout p.cmd.Stderr = p.Stderr From 9b422de60d9a04939d84a21e58464ad53ddc8728 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 17:26:30 +0100 Subject: [PATCH 12/25] Eliminate p.Running and cross-platform Now that the process which starts things is responsible for killing them, it's no longer possible for the *Process to be nil, so the check is unnecessary and there is no platform difference, so it's moved to process.go. --- process.go | 6 ++++-- unix.go | 8 -------- windows.go | 5 ----- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/process.go b/process.go index 30fe207..24de4a9 100644 --- a/process.go +++ b/process.go @@ -4,6 +4,7 @@ import ( "io" "os" "os/exec" + "syscall" ) type Process struct { @@ -30,8 +31,9 @@ func NewProcess(command string, env Env) (p *Process) { return } -func (p *Process) Running() bool { - return (p.cmd.Process != nil) +func (p *Process) Signal(signal syscall.Signal) { + group, _ := os.FindProcess(-1 * p.Pid()) + group.Signal(signal) } func (p *Process) Pid() int { diff --git a/unix.go b/unix.go index 14cdbdf..46c88e3 100644 --- a/unix.go +++ b/unix.go @@ -4,7 +4,6 @@ package main import ( "fmt" - "os" "os/exec" "path/filepath" "syscall" @@ -27,13 +26,6 @@ func (p *Process) Start() { p.cmd.Start() } -func (p *Process) Signal(signal syscall.Signal) { - if p.Running() { - group, _ := os.FindProcess(-1 * p.Pid()) - group.Signal(signal) - } -} - func (p *Process) SendSigTerm() { p.Signal(syscall.SIGTERM) } diff --git a/windows.go b/windows.go index a45a609..bfb1b4b 100644 --- a/windows.go +++ b/windows.go @@ -22,11 +22,6 @@ func (p *Process) Start() { p.cmd.Start() } -func (p *Process) Signal(signal syscall.Signal) { - group, _ := os.FindProcess(-1 * p.cmd.Process.Pid) - group.Signal(signal) -} - func (p *Process) SendSigTerm() { panic("SendSigTerm() not implemented on this platform") } From 708f015f2649201438d28f5589333f14f59e23e9 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 17:58:59 +0100 Subject: [PATCH 13/25] Refactor ShellInvocationCommand This is done because I want to make the *exec.Command shim narrower so that we can transition to another method for reading Stdout/Stderr to fix the newline bug described in #16. --- process.go | 8 -------- unix.go | 13 ++++++++++++- windows.go | 6 +++++- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/process.go b/process.go index 24de4a9..7781727 100644 --- a/process.go +++ b/process.go @@ -43,11 +43,3 @@ func (p *Process) Pid() int { func (p *Process) Wait() { p.cmd.Wait() } - -func (p *Process) shellArgument() string { - if p.Interactive { - return "-ic" - } else { - return "-c" - } -} diff --git a/unix.go b/unix.go index 46c88e3..95f953f 100644 --- a/unix.go +++ b/unix.go @@ -11,8 +11,19 @@ import ( const osHaveSigTerm = true +func ShellInvocationCommand(interactive bool, root, command string) []string { + shellArgument := "-c" + if interactive { + shellArgument = "-ic" + } + profile := filepath.Join(root, ".profile") + shellCommand := fmt.Sprintf("source \"%s\" 2>/dev/null; %s", profile, command) + return []string{"/bin/bash", shellArgument, shellCommand} + +} + func (p *Process) Start() { - command := []string{"/bin/bash", p.shellArgument(), fmt.Sprintf("source \"%s\" 2>/dev/null; %s", filepath.Join(p.Root, ".profile"), p.Command)} + command := ShellInvocationCommand(p.Interactive, p.Root, p.Command) p.cmd = exec.Command(command[0], command[1:]...) p.cmd.Dir = p.Root p.cmd.Env = p.Env.asArray() diff --git a/windows.go b/windows.go index bfb1b4b..cf57bb5 100644 --- a/windows.go +++ b/windows.go @@ -11,8 +11,12 @@ import ( const osHaveSigTerm = false +func ShellInvocationCommand(interactive bool, root, command string) []string { + return []string{"cmd", "/C", command} +} + func (p *Process) Start() { - command := []string{"cmd", "/C", p.Command} + command := ShellInvocationCommand(p.Root, p.Command) p.cmd = exec.Command(command[0], command[1:]...) p.cmd.Dir = p.Root p.cmd.Env = p.Env.asArray() From 9fa9adb06df31cf1ba93f4d60d9269aa448bbe98 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 19:05:24 +0100 Subject: [PATCH 14/25] Factor out PlatformSpecificInit() --- unix.go | 13 +++++++++---- windows.go | 5 +++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/unix.go b/unix.go index 95f953f..ff764b9 100644 --- a/unix.go +++ b/unix.go @@ -22,6 +22,14 @@ func ShellInvocationCommand(interactive bool, root, command string) []string { } +func (p *Process) PlatformSpecificInit() { + if !p.Interactive { + p.cmd.SysProcAttr = &syscall.SysProcAttr{} + p.cmd.SysProcAttr.Setsid = true + } + return +} + func (p *Process) Start() { command := ShellInvocationCommand(p.Interactive, p.Root, p.Command) p.cmd = exec.Command(command[0], command[1:]...) @@ -30,10 +38,7 @@ func (p *Process) Start() { p.cmd.Stdin = p.Stdin p.cmd.Stdout = p.Stdout p.cmd.Stderr = p.Stderr - if !p.Interactive { - p.cmd.SysProcAttr = &syscall.SysProcAttr{} - p.cmd.SysProcAttr.Setsid = true - } + p.PlatformSpecificInit() p.cmd.Start() } diff --git a/windows.go b/windows.go index cf57bb5..8155b80 100644 --- a/windows.go +++ b/windows.go @@ -15,6 +15,11 @@ func ShellInvocationCommand(interactive bool, root, command string) []string { return []string{"cmd", "/C", command} } +func (p *Process) PlatformSpecificInit() { + // NOP on windows for now. + return +} + func (p *Process) Start() { command := ShellInvocationCommand(p.Root, p.Command) p.cmd = exec.Command(command[0], command[1:]...) From 537f1d6e4091b484b19792b5b967943a1ba290ec Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 19:07:05 +0100 Subject: [PATCH 15/25] Make Process.Start the same across platforms --- process.go | 12 ++++++++++++ unix.go | 13 ------------- windows.go | 11 ----------- 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/process.go b/process.go index 7781727..73a5b49 100644 --- a/process.go +++ b/process.go @@ -31,6 +31,18 @@ func NewProcess(command string, env Env) (p *Process) { return } +func (p *Process) Start() { + command := ShellInvocationCommand(p.Interactive, p.Root, p.Command) + p.cmd = exec.Command(command[0], command[1:]...) + p.cmd.Dir = p.Root + p.cmd.Env = p.Env.asArray() + p.cmd.Stdin = p.Stdin + p.cmd.Stdout = p.Stdout + p.cmd.Stderr = p.Stderr + p.PlatformSpecificInit() + p.cmd.Start() +} + func (p *Process) Signal(signal syscall.Signal) { group, _ := os.FindProcess(-1 * p.Pid()) group.Signal(signal) diff --git a/unix.go b/unix.go index ff764b9..f691293 100644 --- a/unix.go +++ b/unix.go @@ -4,7 +4,6 @@ package main import ( "fmt" - "os/exec" "path/filepath" "syscall" ) @@ -30,18 +29,6 @@ func (p *Process) PlatformSpecificInit() { return } -func (p *Process) Start() { - command := ShellInvocationCommand(p.Interactive, p.Root, p.Command) - p.cmd = exec.Command(command[0], command[1:]...) - p.cmd.Dir = p.Root - p.cmd.Env = p.Env.asArray() - p.cmd.Stdin = p.Stdin - p.cmd.Stdout = p.Stdout - p.cmd.Stderr = p.Stderr - p.PlatformSpecificInit() - p.cmd.Start() -} - func (p *Process) SendSigTerm() { p.Signal(syscall.SIGTERM) } diff --git a/windows.go b/windows.go index 8155b80..e902105 100644 --- a/windows.go +++ b/windows.go @@ -20,17 +20,6 @@ func (p *Process) PlatformSpecificInit() { return } -func (p *Process) Start() { - command := ShellInvocationCommand(p.Root, p.Command) - p.cmd = exec.Command(command[0], command[1:]...) - p.cmd.Dir = p.Root - p.cmd.Env = p.Env.asArray() - p.cmd.Stdin = p.Stdin - p.cmd.Stdout = p.Stdout - p.cmd.Stderr = p.Stderr - p.cmd.Start() -} - func (p *Process) SendSigTerm() { panic("SendSigTerm() not implemented on this platform") } From a899cb4fed68be01367c513f7d5249822b8db365 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 19:10:17 +0100 Subject: [PATCH 16/25] Don't set p.Root in NewProcess It's being eliminated by embedding the *Cmd in the Process. --- process.go | 1 - run.go | 8 ++++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/process.go b/process.go index 73a5b49..9f3e391 100644 --- a/process.go +++ b/process.go @@ -27,7 +27,6 @@ func NewProcess(command string, env Env) (p *Process) { p.Stdin = os.Stdin p.Stdout = os.Stdout p.Stderr = os.Stderr - p.Root, _ = os.Getwd() return } diff --git a/run.go b/run.go index a230a44..6b2d5dd 100644 --- a/run.go +++ b/run.go @@ -24,9 +24,12 @@ func init() { } func runRun(cmd *Command, args []string) { + workDir, err := os.Getwd() + if err != nil { + handleError(err) + } if flagEnv == "" { - root, _ := os.Getwd() - flagEnv = filepath.Join(root, ".env") + flagEnv = filepath.Join(workDir, ".env") } env, err := ReadEnv(flagEnv) @@ -34,6 +37,7 @@ func runRun(cmd *Command, args []string) { ps := NewProcess(strings.Join(args, " "), env) ps.Interactive = true + ps.Root = workDir ps.Stdin = os.Stdin ps.Stdout = os.Stdout ps.Stderr = os.Stderr From c165325ed1850eb3678ff0ac5d37cc370a8b6cc5 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 19:24:11 +0100 Subject: [PATCH 17/25] Embed *exec.Cmd in Process struct Motivation: Trying to slim the Process wrapper as much as possible to make later transformations easier. --- process.go | 39 ++++++++++----------------------------- run.go | 13 ++++++++----- start.go | 7 ++++--- unix.go | 4 ++-- 4 files changed, 24 insertions(+), 39 deletions(-) diff --git a/process.go b/process.go index 9f3e391..ff8aa47 100644 --- a/process.go +++ b/process.go @@ -1,7 +1,6 @@ package main import ( - "io" "os" "os/exec" "syscall" @@ -11,35 +10,21 @@ type Process struct { Command string Env Env Interactive bool - Stdin io.Reader - Stdout io.Writer - Stderr io.Writer - Root string - cmd *exec.Cmd + *exec.Cmd } -func NewProcess(command string, env Env) (p *Process) { - p = new(Process) - p.Command = command - p.Env = env - p.Interactive = false - p.Stdin = os.Stdin - p.Stdout = os.Stdout - p.Stderr = os.Stderr - return +func NewProcess(workdir, command string, env Env, interactive bool) (p *Process) { + argv := ShellInvocationCommand(interactive, workdir, command) + return &Process{ + command, env, interactive, exec.Command(argv[0], argv[1:]...), + } } -func (p *Process) Start() { - command := ShellInvocationCommand(p.Interactive, p.Root, p.Command) - p.cmd = exec.Command(command[0], command[1:]...) - p.cmd.Dir = p.Root - p.cmd.Env = p.Env.asArray() - p.cmd.Stdin = p.Stdin - p.cmd.Stdout = p.Stdout - p.cmd.Stderr = p.Stderr +func (p *Process) Start() error { + p.Cmd.Env = p.Env.asArray() p.PlatformSpecificInit() - p.cmd.Start() + return p.Cmd.Start() } func (p *Process) Signal(signal syscall.Signal) { @@ -48,9 +33,5 @@ func (p *Process) Signal(signal syscall.Signal) { } func (p *Process) Pid() int { - return p.cmd.Process.Pid -} - -func (p *Process) Wait() { - p.cmd.Wait() + return p.Process.Pid } diff --git a/run.go b/run.go index 6b2d5dd..d7f067d 100644 --- a/run.go +++ b/run.go @@ -35,12 +35,15 @@ func runRun(cmd *Command, args []string) { env, err := ReadEnv(flagEnv) handleError(err) - ps := NewProcess(strings.Join(args, " "), env) - ps.Interactive = true - ps.Root = workDir + const interactive = true + ps := NewProcess(workDir, strings.Join(args, " "), env, interactive) ps.Stdin = os.Stdin ps.Stdout = os.Stdout ps.Stderr = os.Stderr - ps.Start() - ps.Wait() + + err = ps.Start() + handleError(err) + + err = ps.Wait() + handleError(err) } diff --git a/start.go b/start.go index a40dc75..1b1a72c 100644 --- a/start.go +++ b/start.go @@ -109,10 +109,11 @@ func (f *Forego) monitorInterrupt() { func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of *OutletFactory) { port := flagPort + (idx * 100) - ps := NewProcess(proc.Command, env) + const interactive = false + workDir := filepath.Dir(flagProcfile) + ps := NewProcess(workDir, proc.Command, env, interactive) procName := fmt.Sprint(proc.Name, ".", procNum+1) ps.Env["PORT"] = strconv.Itoa(port) - ps.Root = filepath.Dir(flagProcfile) ps.Stdin = nil ps.Stdout = of.CreateOutlet(procName, idx, false) ps.Stderr = of.CreateOutlet(procName, idx, true) @@ -148,7 +149,7 @@ func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of if !osHaveSigTerm { of.SystemOutput(fmt.Sprintf("Killing %s", procName)) - ps.cmd.Process.Kill() + ps.Process.Kill() return } diff --git a/unix.go b/unix.go index f691293..7f31bfe 100644 --- a/unix.go +++ b/unix.go @@ -23,8 +23,8 @@ func ShellInvocationCommand(interactive bool, root, command string) []string { func (p *Process) PlatformSpecificInit() { if !p.Interactive { - p.cmd.SysProcAttr = &syscall.SysProcAttr{} - p.cmd.SysProcAttr.Setsid = true + p.SysProcAttr = &syscall.SysProcAttr{} + p.SysProcAttr.Setsid = true } return } From 16953497ba37bc23263702b0b8d23c00c7af29a2 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 19:26:42 +0100 Subject: [PATCH 18/25] Eliminate unused imports from windows.go --- windows.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/windows.go b/windows.go index e902105..446c543 100644 --- a/windows.go +++ b/windows.go @@ -3,9 +3,6 @@ package main // +build windows import ( - "fmt" - "os" - "os/exec" "syscall" ) From 1b47f052247474132a0bd92ed4e918bc55bc2e52 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 19:30:22 +0100 Subject: [PATCH 19/25] Eliminate Pid() function --- process.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/process.go b/process.go index ff8aa47..f575a8c 100644 --- a/process.go +++ b/process.go @@ -28,10 +28,6 @@ func (p *Process) Start() error { } func (p *Process) Signal(signal syscall.Signal) { - group, _ := os.FindProcess(-1 * p.Pid()) + group, _ := os.FindProcess(-1 * p.Process.Pid) group.Signal(signal) } - -func (p *Process) Pid() int { - return p.Process.Pid -} From 78d368d8aed464fa5e2e2a4352fd2e61ada4301f Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 19:31:38 +0100 Subject: [PATCH 20/25] Add error handling to Signal() --- process.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/process.go b/process.go index f575a8c..ae1bcdd 100644 --- a/process.go +++ b/process.go @@ -27,7 +27,10 @@ func (p *Process) Start() error { return p.Cmd.Start() } -func (p *Process) Signal(signal syscall.Signal) { - group, _ := os.FindProcess(-1 * p.Process.Pid) - group.Signal(signal) +func (p *Process) Signal(signal syscall.Signal) error { + group, err := os.FindProcess(-1 * p.Process.Pid) + if err == nil { + err = group.Signal(signal) + } + return err } From 34b39bf589fc727164368695be1aa908972de324 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 20:26:56 +0100 Subject: [PATCH 21/25] Fix #16 and race condition on final lines printed This commit refactors to read whole lines from Stdin/Stdout pipes and fixes a race condition where the final output of a process may be missed. --- outlet.go | 20 +++++++++++++------- start.go | 31 ++++++++++++++++++++++++------- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/outlet.go b/outlet.go index df2ea45..95d3ad4 100644 --- a/outlet.go +++ b/outlet.go @@ -11,7 +11,6 @@ import ( ) type OutletFactory struct { - Outlets map[string]*Outlet Padding int sync.Mutex @@ -34,9 +33,7 @@ var colors = []ct.Color{ } func NewOutletFactory() (of *OutletFactory) { - of = new(OutletFactory) - of.Outlets = make(map[string]*Outlet) - return + return new(OutletFactory) } func (o *Outlet) Write(b []byte) (num int, err error) { @@ -52,9 +49,15 @@ func ProcessOutput(w io.Writer, str string) { w.Write([]byte(str)) } -func (of *OutletFactory) CreateOutlet(name string, index int, isError bool) *Outlet { - of.Outlets[name] = &Outlet{name, colors[index%len(colors)], isError, of} - return of.Outlets[name] +func (of *OutletFactory) LineReader(wg *sync.WaitGroup, name string, index int, r io.Reader, isError bool) { + defer wg.Done() + + o := &Outlet{name, colors[index%len(colors)], isError, of} + + scanner := bufio.NewScanner(r) + for scanner.Scan() { + of.WriteLine(o.Name, scanner.Text(), o.Color, ct.None, o.IsError) + } } func (of *OutletFactory) SystemOutput(str string) { @@ -81,4 +84,7 @@ func (of *OutletFactory) WriteLine(left, right string, leftC, rightC ct.Color, i ct.ResetColor() } fmt.Println(right) + if isError { + ct.ResetColor() + } } diff --git a/start.go b/start.go index 1b1a72c..e836af1 100644 --- a/start.go +++ b/start.go @@ -71,9 +71,10 @@ func parseConcurrency(value string) (map[string]int, error) { type Forego struct { outletFactory *OutletFactory - shutdown sync.Once // Closes teardown exactly once - teardown chan struct{} // barrier: closed when shutting down - teardownNow chan struct{} // barrier: second CTRL-C. More urgent. + + shutdown sync.Once // Closes teardown exactly once + teardown chan struct{} // barrier: closed when shutting down + teardownNow chan struct{} // barrier: second CTRL-C. More urgent. wg sync.WaitGroup } @@ -114,17 +115,34 @@ func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of ps := NewProcess(workDir, proc.Command, env, interactive) procName := fmt.Sprint(proc.Name, ".", procNum+1) ps.Env["PORT"] = strconv.Itoa(port) + ps.Stdin = nil - ps.Stdout = of.CreateOutlet(procName, idx, false) - ps.Stderr = of.CreateOutlet(procName, idx, true) + + stdout, err := ps.StdoutPipe() + if err != nil { + panic(err) + } + stderr, err := ps.StderrPipe() + if err != nil { + panic(err) + } + + pipeWait := new(sync.WaitGroup) + pipeWait.Add(2) + go of.LineReader(pipeWait, procName, idx, stdout, false) + go of.LineReader(pipeWait, procName, idx, stderr, true) of.SystemOutput(fmt.Sprintf("starting %s on port %d", procName, port)) finished := make(chan struct{}) // closed on process exit ps.Start() + + f.wg.Add(1) go func() { + defer f.wg.Done() defer close(finished) + pipeWait.Wait() ps.Wait() }() @@ -134,14 +152,13 @@ func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of // Prevent goroutine from exiting before process has finished. defer func() { <-finished }() + defer f.SignalShutdown() select { case <-finished: if flagRestart { f.startProcess(idx, procNum, proc, env, of) return - } else { - f.SignalShutdown() } case <-f.teardown: From 8b69d28b6664c3e12a019c236e8f11f470f053d2 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 20:53:09 +0100 Subject: [PATCH 22/25] Eliminate unused functions and type Outlet --- outlet.go | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/outlet.go b/outlet.go index 95d3ad4..946c485 100644 --- a/outlet.go +++ b/outlet.go @@ -2,7 +2,6 @@ package main import ( "bufio" - "bytes" "fmt" "github.com/daviddengcn/go-colortext" "io" @@ -16,13 +15,6 @@ type OutletFactory struct { sync.Mutex } -type Outlet struct { - Name string - Color ct.Color - IsError bool - Factory *OutletFactory -} - var colors = []ct.Color{ ct.Cyan, ct.Yellow, @@ -36,27 +28,14 @@ func NewOutletFactory() (of *OutletFactory) { return new(OutletFactory) } -func (o *Outlet) Write(b []byte) (num int, err error) { - scanner := bufio.NewScanner(bytes.NewReader(b)) - for scanner.Scan() { - o.Factory.WriteLine(o.Name, scanner.Text(), ct.White, ct.None, o.IsError) - } - num = len(b) - return -} - -func ProcessOutput(w io.Writer, str string) { - w.Write([]byte(str)) -} - func (of *OutletFactory) LineReader(wg *sync.WaitGroup, name string, index int, r io.Reader, isError bool) { defer wg.Done() - o := &Outlet{name, colors[index%len(colors)], isError, of} + color := colors[index%len(colors)] scanner := bufio.NewScanner(r) for scanner.Scan() { - of.WriteLine(o.Name, scanner.Text(), o.Color, ct.None, o.IsError) + of.WriteLine(name, scanner.Text(), color, ct.None, isError) } } From 2c17c9a4899a565e51b26140dabfaaea022e80c8 Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Wed, 21 May 2014 22:00:12 +0100 Subject: [PATCH 23/25] Add writey fixture program --- fixtures/writey/Procfile | 2 ++ fixtures/writey/main.go | 42 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 fixtures/writey/Procfile create mode 100644 fixtures/writey/main.go diff --git a/fixtures/writey/Procfile b/fixtures/writey/Procfile new file mode 100644 index 0000000..6e91b25 --- /dev/null +++ b/fixtures/writey/Procfile @@ -0,0 +1,2 @@ +writey1: writey +writey2: writey diff --git a/fixtures/writey/main.go b/fixtures/writey/main.go new file mode 100644 index 0000000..ff652b5 --- /dev/null +++ b/fixtures/writey/main.go @@ -0,0 +1,42 @@ +package main + +import ( + "fmt" + "math/rand" + "os" + "os/signal" + "syscall" + "time" +) + +func main() { + rand.Seed(time.Now().UnixNano()) + print("Foo") + time.Sleep(10 * time.Millisecond) + println("Bar") + + print("Baz") + time.Sleep(10 * time.Millisecond) + println("Qux") + + fmt.Fprintln(os.Stdout, "This is on \x1b[32mstdout") + + os.Stdout.Close() + + s := rand.Intn(3) + 1 + + c := make(chan os.Signal) + signal.Notify(c, os.Interrupt, syscall.SIGTERM) + + select { + case <-c: + if rand.Intn(4) == 1 { + println("IGNORING EXIT") + time.Sleep(100 * time.Second) + } + println("Got SIGTERM") + case <-time.After(time.Duration(s) * time.Second): + println("Timed out") + } + +} From 08178d82bd987d0676faf0e7104a318e02c52f0c Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Thu, 22 May 2014 07:56:00 +0100 Subject: [PATCH 24/25] Fix windows build --- unix.go | 4 ++-- windows.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/unix.go b/unix.go index 7f31bfe..1cf106b 100644 --- a/unix.go +++ b/unix.go @@ -1,7 +1,7 @@ -package main - // +build darwin freebsd linux netbsd openbsd +package main + import ( "fmt" "path/filepath" diff --git a/windows.go b/windows.go index 446c543..232c4bc 100644 --- a/windows.go +++ b/windows.go @@ -1,7 +1,7 @@ -package main - // +build windows +package main + import ( "syscall" ) From 37aac8a97eaea61d07a3d05534b77d39d79d488d Mon Sep 17 00:00:00 2001 From: Peter Waller Date: Thu, 22 May 2014 17:40:09 +0100 Subject: [PATCH 25/25] Refactor to use Barrier --- barrier.go | 37 +++++++++++++++++++++++++++++++++++++ start.go | 39 ++++++++++++++++----------------------- 2 files changed, 53 insertions(+), 23 deletions(-) create mode 100644 barrier.go diff --git a/barrier.go b/barrier.go new file mode 100644 index 0000000..3709721 --- /dev/null +++ b/barrier.go @@ -0,0 +1,37 @@ +package main + +import ( + "sync" +) + +// Direct import of https://github.com/pwaller/barrier/blob/master/barrier.go + +// The zero of Barrier is a ready-to-use value +type Barrier struct { + channel chan struct{} + fall, initialize sync.Once + FallHook func() +} + +func (b *Barrier) init() { + b.initialize.Do(func() { b.channel = make(chan struct{}) }) +} + +// `b.Fall()` can be called any number of times and causes the channel returned +// by `b.Barrier()` to become closed (permanently available for immediate reading) +func (b *Barrier) Fall() { + b.init() + b.fall.Do(func() { + if b.FallHook != nil { + b.FallHook() + } + close(b.channel) + }) +} + +// When `b.Fall()` is called, the channel returned by Barrier() is closed +// (and becomes always readable) +func (b *Barrier) Barrier() <-chan struct{} { + b.init() + return b.channel +} diff --git a/start.go b/start.go index e836af1..d63fd76 100644 --- a/start.go +++ b/start.go @@ -72,36 +72,26 @@ func parseConcurrency(value string) (map[string]int, error) { type Forego struct { outletFactory *OutletFactory - shutdown sync.Once // Closes teardown exactly once - teardown chan struct{} // barrier: closed when shutting down - teardownNow chan struct{} // barrier: second CTRL-C. More urgent. + teardown, teardownNow Barrier // signal shutting down wg sync.WaitGroup } -func (f *Forego) SignalShutdown() { - f.shutdown.Do(func() { - f.outletFactory.SystemOutput("shutting down") - close(f.teardown) - }) -} - func (f *Forego) monitorInterrupt() { handler := make(chan os.Signal, 1) signal.Notify(handler, os.Interrupt) first := true - var once sync.Once for sig := range handler { switch sig { case os.Interrupt: fmt.Println(" | ctrl-c detected") + f.teardown.Fall() if !first { - once.Do(func() { close(f.teardownNow) }) + f.teardownNow.Fall() } - f.SignalShutdown() first = false } } @@ -152,7 +142,7 @@ func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of // Prevent goroutine from exiting before process has finished. defer func() { <-finished }() - defer f.SignalShutdown() + defer f.teardown.Fall() select { case <-finished: @@ -161,7 +151,7 @@ func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of return } - case <-f.teardown: + case <-f.teardown.Barrier(): // Forego tearing down if !osHaveSigTerm { @@ -175,10 +165,7 @@ func (f *Forego) startProcess(idx, procNum int, proc ProcfileEntry, env Env, of // Give the process a chance to exit, otherwise kill it. select { - case <-time.After(shutdownGraceTime): - of.SystemOutput(fmt.Sprintf("Killing %s", procName)) - ps.SendSigKill() - case <-f.teardownNow: + case <-f.teardownNow.Barrier(): of.SystemOutput(fmt.Sprintf("Killing %s", procName)) ps.SendSigKill() case <-finished: @@ -208,13 +195,19 @@ func runStart(cmd *Command, args []string) { f := &Forego{ outletFactory: of, - - teardown: make(chan struct{}), - teardownNow: make(chan struct{}), } go f.monitorInterrupt() + // When teardown fires, start the grace timer + f.teardown.FallHook = func() { + go func() { + time.Sleep(shutdownGraceTime) + of.SystemOutput("Grace time expired") + f.teardownNow.Fall() + }() + } + var singleton string = "" if len(args) > 0 { singleton = args[0] @@ -235,7 +228,7 @@ func runStart(cmd *Command, args []string) { } } - <-f.teardown + <-f.teardown.Barrier() f.wg.Wait() }