Skip to content

Commit

Permalink
Default to SIGTERM and fix SIGINT handling
Browse files Browse the repository at this point in the history
go 1.11 added signal.Ignored \o/
  • Loading branch information
rwstauner committed Oct 14, 2018
1 parent fd7540e commit 703dbb3
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 18 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
# v0.10 (2018-10-14)

- Use SIGTERM as default stop signal.
- Do not handle signals that are already ignored
(for example: SIGINT when not run in the foreground).
- Make starting/stopping output simpler and more consistent.

# v0.9 (2018-03-19)

- Remove deprecated -listen option
Expand Down
2 changes: 1 addition & 1 deletion README.mkdn
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ One service can be configured on the command line:
-proxy # Address pairs to listen on/proxy to ("from:port to:port from2 to2")
-proxy-sep # Alternate character to separate proxy addresses
-stop-after # Duration after last connection to signal command to stop
-stop-signal # Signal to send to command (default it INT)
-stop-signal # Signal to send to command (default is TERM)
-timeout # Duration to wait before aborting connection
-wait-after-start # Duration to wait after starting command before forwarding connections
args... # Command to run
Expand Down
2 changes: 1 addition & 1 deletion config/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ var proxySep = " "
var proxySpec string
var timeout = DefaultTimeout
var stopAfter = DefaultStopAfter
var stopSignal = "INT"
var stopSignal = "TERM"
var waitAfterStart = DefaultWaitAfterStart

func init() {
Expand Down
8 changes: 4 additions & 4 deletions config/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,28 +189,28 @@ func TestLoadStopAfter(t *testing.T) {

t.Run("default", func(t *testing.T) {
stopAfter = 0
stopSignal = "INT"
stopSignal = "TERM"

svc := setup(t)

if svc.StopAfter != "0s" {
t.Errorf("incorrect StopAfter: %q", svc.StopAfter)
}
if svc.StopSignal != "INT" {
if svc.StopSignal != "TERM" {
t.Errorf("incorrect StopAfter: %q", svc.StopSignal)
}
})

t.Run("custom", func(t *testing.T) {
stopAfter = 200 * time.Millisecond
stopSignal = "TERM"
stopSignal = "INT"

svc := setup(t)

if svc.StopAfter != "200ms" {
t.Errorf("incorrect StopAfter: %q", svc.StopAfter)
}
if svc.StopSignal != "TERM" {
if svc.StopSignal != "INT" {
t.Errorf("incorrect StopAfter: %q", svc.StopSignal)
}
})
Expand Down
2 changes: 1 addition & 1 deletion procman/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (m *ProcessManager) Process(cfg config.Service) *Process {
return &Process{
argv: cfg.Command,
manager: m,
stopSignal: getSignal(cfg.StopSignal, syscall.SIGINT),
stopSignal: getSignal(cfg.StopSignal, syscall.SIGTERM),
waitAfterStart: config.ParseDuration(cfg.WaitAfterStart, config.DefaultWaitAfterStart),
}
}
Expand Down
8 changes: 3 additions & 5 deletions procman/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (
"github.com/rwstauner/ynetd/config"
)

var stopSignal = syscall.SIGINT

func TestProcess(t *testing.T) {
pm := New()
proc := pm.Process(config.Service{Command: []string{"foo", "bar"}})
Expand All @@ -20,7 +18,7 @@ func TestProcess(t *testing.T) {
if proc.manager != pm {
t.Errorf("who dis?")
}
if proc.stopSignal != syscall.SIGINT {
if proc.stopSignal != syscall.SIGTERM {
t.Errorf("incorrect stop signal: %s", proc.stopSignal)
}
}
Expand All @@ -29,13 +27,13 @@ func TestProcessStopSignal(t *testing.T) {
pm := New()
proc := pm.Process(config.Service{
Command: []string{"siggy"},
StopSignal: "TERM",
StopSignal: "INT",
})

if fmt.Sprintf("%s", proc.argv) != "[siggy]" {
t.Errorf("Unexpected argv: %s", proc.argv)
}
if proc.stopSignal != syscall.SIGTERM {
if proc.stopSignal != syscall.SIGINT {
t.Errorf("incorrect stop signal: %s", proc.stopSignal)
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/helpers.bash
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ ysend () {
close () {
if [[ -n "$YPID" ]]; then
# Don't count these exit statuses as errors.
kill -s INT $YPID || :
kill $YPID || :
wait $YPID || :
fi
YPID=
Expand Down
29 changes: 28 additions & 1 deletion test/signals.bats
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ ystate () {
ytester -loop -serve "wave$YTAG"
running ynetd
! running ytester
kill -s INT $YPID
kill $YPID
sleep 1 # With -race this takes slightly longer.
! running ynetd
}
Expand All @@ -27,6 +27,33 @@ ystate () {
is `ystate` = S
}

@test "ignores INT when already ignored" {
"$YAS" "ynetd$YTAG" "$YNETD" -proxy "localhost:$LISTEN_PORT localhost:$PROXY_PORT" &
YPID=$!
running ynetd

kill -s INT $YPID
sleep 1
running ynetd

kill -s TERM $YPID
sleep 1
! running ynetd
}

@test "respects INT in the foreground" {
# For the default bash (3.2) on MacOS (High Sierra)
# this is true when running more than one bats file (bats-exec-suite).
if "$YTESTER" -int-ignored 2>&1 | grep -q "int ignored: true"; then
skip "SIGINT is already ignored"
fi

(pid=; while [[ -z "$pid" ]]; do sleep 1; pid=`ypidof ynetd`; done; kill -s INT "$pid") & # bg
"$YAS" "ynetd$YTAG" "$YNETD" -proxy "localhost:$LISTEN_PORT localhost:$PROXY_PORT" # fg
# First (bg) proc should kill the second (fg) one.
! running ynetd
}

@test "signal process group" {
ynetd -proxy "localhost:$LISTEN_PORT localhost:$PROXY_PORT" -stop-after 2s \
"$YAS" "ytester$YTAG" \
Expand Down
2 changes: 1 addition & 1 deletion test/wait_after_start.bats
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ load helpers

is $((end - start)) -lt 2

kill -s INT `ypidof ytester`
kill `ypidof ytester`
! running ytester

# Each new start should wait.
Expand Down
10 changes: 8 additions & 2 deletions test/ytester.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ import (
"log"
"net"
"os"
"os/signal"
"syscall"
"time"
)

var (
after = 0 * time.Millisecond
before = 0 * time.Millisecond
intIgnored = false
knock = false
loop = false
port = ""
Expand All @@ -27,6 +30,7 @@ func init() {
flag.DurationVar(&before, "before", before, "before")
flag.BoolVar(&knock, "knock", knock, "knock")
flag.BoolVar(&loop, "loop", loop, "loop")
flag.BoolVar(&intIgnored, "int-ignored", intIgnored, "int-ignored")
flag.StringVar(&port, "port", port, "port")
flag.StringVar(&send, "send", send, "send")
flag.StringVar(&serve, "serve", serve, "serve")
Expand Down Expand Up @@ -80,14 +84,16 @@ func frd() int {
time.Sleep(before)

switch {
case intIgnored:
flog("int ignored: %t", signal.Ignored(syscall.SIGINT))
case knock:
flog("knocking %d", port)
flog("knocking %s", port)
net.Dial("tcp", "localhost:"+port)
case send != "":
c := make(chan net.Conn)
go func() {
for {
flog("dialing %d", port)
flog("dialing %s", port)
conn, err := net.DialTimeout("tcp", "localhost:"+port, timeout)
if err == nil {
c <- conn
Expand Down
7 changes: 6 additions & 1 deletion ynetd.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ var logger = log.New(os.Stdout, "ynetd ", log.Ldate|log.Ltime|log.Lmicroseconds)
func setupSignals(pm *procman.ProcessManager) {
channel := make(chan os.Signal, 1)

signal.Notify(channel, syscall.SIGHUP, syscall.SIGINT, syscall.SIGTERM)
signals := []os.Signal{syscall.SIGHUP, syscall.SIGINT, syscall.SIGTERM}
for _, sig := range signals {
if !signal.Ignored(sig) {
signal.Notify(channel, sig)
}
}

pm.Signal(<-channel)
}
Expand Down

0 comments on commit 703dbb3

Please sign in to comment.