Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/signal: signal.Ignore is inherited by children after a forkExec #20479

Closed
muhamadazmy opened this issue May 24, 2017 · 3 comments
Closed

os/signal: signal.Ignore is inherited by children after a forkExec #20479

muhamadazmy opened this issue May 24, 2017 · 3 comments
Milestone

Comments

@muhamadazmy
Copy link

What version of Go are you using (go version)?

go version go1.8.1 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/azmy/work/go"
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build637832860=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

package main

import (
	"fmt"
	"os/exec"
	"os/signal"
	"syscall"
	"time"
)

func terminate(pid int) {
	for {
		<-time.After(1 * time.Second)
		fmt.Println("trying to kill")
		syscall.Kill(pid, syscall.SIGTERM)
	}
}

func main() {
	signal.Ignore(syscall.SIGTERM)
	cmd := exec.Command("sleep", "3")

	if err := cmd.Start(); err != nil {
		panic(err)
	}

	fmt.Println("PID:", cmd.Process.Pid)
	go terminate(cmd.Process.Pid)
	if state, err := cmd.Process.Wait(); err != nil {
		panic(err)
	} else {
		fmt.Println("Success:", state.Success(), state)
	}
}

What did you expect to see?

When run the program should print

PID: <pid>
trying to kill
Success: false signal: terminated

A signal terminate to the sleep process should still kill it since execve system call should reset all signal handlers to the default ones. But for some reason this doesn't happen with ignore. To work around this I had to add empty signal handler for the terminate signal so only the parent process ignore the signal while the children still have their default handler in place.

What did you see instead?

PID: <pid>
trying to kill
trying to kill
trying to kill
Success: true exit status 0
@bradfitz bradfitz changed the title signal.Ignore is inherited by children after a forkExec os/signal: signal.Ignore is inherited by children after a forkExec May 24, 2017
@bradfitz
Copy link
Contributor

/cc @ianlancetaylor @bcmills

@bradfitz bradfitz added this to the Go1.10 milestone May 24, 2017
@ianlancetaylor
Copy link
Contributor

POSIX specifies that for execve signals whose handlers are either SIG_IGN or SIG_DFL are left unchanged: that is, an ignored signal is still ignored after an execve. Calling signal.Ignore sets the handler to SIG_IGN, so what are you seeing is that the execution is preserving the ignored state of the signal. Go doesn't have to follow POSIX, of course, but you are basically suggesting that os.StartProcess should override ignored signals and set them back to the default. We could do that, but is it the right thing to do?

There is a clear utility to being able to ignore signals across execve: that's how the nohup utility works. Today, we could write the nohup utility in Go. With your proposed change, we would not be able to. It's easy to catch but ignore a signal in Go: just call signal.Notify(make(chan os.Signal)). So your proposed change would make Go strictly less useful than it is today. That does not seem wise.

One possibility would be to add something to syscall.SysProcAttr that lets you specify signal dispositions (either ignored or default). Then you could easily control signals. But given that it is fairly simple to do that anyhow, I'm not sure how useful that would be.

@muhamadazmy
Copy link
Author

@ianlancetaylor thanks for your reply and indeed u have a very good point. Also thanks for the hint how to work around this with signal.Notify.
Feel free to close the issue.

@golang golang locked and limited conversation to collaborators May 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants