daemon: killWithSignal, killPossiblyDeadProcess: accept syscall.Signal

This helps reducing some type-juggling / conversions further up the stack. Signed-off-by: Sebastiaan van Stijn <[email protected]>
rdx0992 · May 4, 2022 · ea1eb44 · ea1eb44
1 parent 2ec2b65
commit ea1eb44
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 20 deletions.
diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
+	"syscall"
 
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/daemon/links"
@@ -336,24 +337,25 @@ func (daemon *Daemon) cleanupSecretDir(c *container.Container) {
 
 func killProcessDirectly(container *container.Container) error {
 	pid := container.GetPID()
-	// Ensure that we don't kill ourselves
 	if pid == 0 {
+		// Ensure that we don't kill ourselves
 		return nil
 	}
 
-	if err := unix.Kill(pid, 9); err != nil {
+	if err := unix.Kill(pid, syscall.SIGKILL); err != nil {
 		if err != unix.ESRCH {
-			return err
+			return errdefs.System(err)
 		}
-		e := errNoSuchProcess{pid, 9}
-		logrus.WithError(e).WithField("container", container.ID).Debug("no such process")
-		return e
+		err = errNoSuchProcess{pid, syscall.SIGKILL}
+		logrus.WithError(err).WithField("container", container.ID).Debug("no such process")
+		return err
 	}
 
 	// In case there were some exceptions(e.g., state of zombie and D)
 	if system.IsProcessAlive(pid) {
 		// Since we can not kill a zombie pid, add zombie check here
 		isZombie, err := system.IsProcessZombie(pid)
+		// TODO(thaJeztah) should we ignore os.IsNotExist() here? ("/proc/<pid>/stat" will be gone if the process exited)
 		if err != nil {
 			logrus.WithError(err).WithField("container", container.ID).Warn("Container state is invalid")
 			return err

diff --git a/daemon/kill.go b/daemon/kill.go
@@ -17,7 +17,7 @@ import (
 
 type errNoSuchProcess struct {
 	pid    int
-	signal int
+	signal syscall.Signal
 }
 
 func (e errNoSuchProcess) Error() string {
@@ -37,30 +37,30 @@ func isErrNoSuchProcess(err error) bool {
 // If no signal is given (sig 0), then Kill with SIGKILL and wait
 // for the container to exit.
 // If a signal is given, then just send it to the container and return.
-func (daemon *Daemon) ContainerKill(name string, sig uint64) error {
+func (daemon *Daemon) ContainerKill(name string, stopSignal uint64) error {
+	sig := syscall.Signal(stopSignal)
 	container, err := daemon.GetContainer(name)
 	if err != nil {
 		return err
 	}
 
-	if sig != 0 && !signal.ValidSignalForPlatform(syscall.Signal(sig)) {
+	if sig != 0 && !signal.ValidSignalForPlatform(sig) {
 		return fmt.Errorf("The %s daemon does not support signal %d", runtime.GOOS, sig)
 	}
 
 	// If no signal is passed, or SIGKILL, perform regular Kill (SIGKILL + wait())
-	if sig == 0 || syscall.Signal(sig) == syscall.SIGKILL {
+	if sig == 0 || sig == syscall.SIGKILL {
 		return daemon.Kill(container)
 	}
-	return daemon.killWithSignal(container, int(sig))
+	return daemon.killWithSignal(container, sig)
 }
 
 // killWithSignal sends the container the given signal. This wrapper for the
 // host specific kill command prepares the container before attempting
 // to send the signal. An error is returned if the container is paused
 // or not running, or if there is a problem returned from the
 // underlying kill command.
-func (daemon *Daemon) killWithSignal(container *containerpkg.Container, sig int) error {
-	var stopSignal = syscall.Signal(sig)
+func (daemon *Daemon) killWithSignal(container *containerpkg.Container, stopSignal syscall.Signal) error {
 	logrus.Debugf("Sending kill signal %d to container %s", stopSignal, container.ID)
 	container.Lock()
 	defer container.Unlock()
@@ -140,7 +140,7 @@ func (daemon *Daemon) Kill(container *containerpkg.Container) error {
 	}
 
 	// 1. Send SIGKILL
-	if err := daemon.killPossiblyDeadProcess(container, int(syscall.SIGKILL)); err != nil {
+	if err := daemon.killPossiblyDeadProcess(container, syscall.SIGKILL); err != nil {
 		// kill failed, check if process is no longer running.
 		if isErrNoSuchProcess(err) {
 			return nil
@@ -175,12 +175,12 @@ func (daemon *Daemon) Kill(container *containerpkg.Container) error {
 }
 
 // killPossibleDeadProcess is a wrapper around killSig() suppressing "no such process" error.
-func (daemon *Daemon) killPossiblyDeadProcess(container *containerpkg.Container, sig int) error {
+func (daemon *Daemon) killPossiblyDeadProcess(container *containerpkg.Container, sig syscall.Signal) error {
 	err := daemon.killWithSignal(container, sig)
 	if errdefs.IsNotFound(err) {
-		e := errNoSuchProcess{container.GetPID(), sig}
-		logrus.Debug(e)
-		return e
+		err = errNoSuchProcess{container.GetPID(), sig}
+		logrus.Debug(err)
+		return err
 	}
 	return err
 }
diff --git a/daemon/stop.go b/daemon/stop.go
@@ -2,6 +2,7 @@ package daemon // import "github.com/docker/docker/daemon"
 
 import (
 	"context"
+	"syscall"
 	"time"
 
 	containertypes "github.com/docker/docker/api/types/container"
@@ -42,15 +43,15 @@ func (daemon *Daemon) containerStop(ctx context.Context, ctr *container.Containe
 	}
 
 	var (
-		stopSignal  = ctr.StopSignal()
+		stopSignal  = syscall.Signal(ctr.StopSignal())
 		stopTimeout = ctr.StopTimeout()
 	)
 	if options.Signal != "" {
 		sig, err := signal.ParseSignal(options.Signal)
 		if err != nil {
 			return errdefs.InvalidParameter(err)
 		}
-		stopSignal = int(sig)
+		stopSignal = sig
 	}
 	if options.Timeout != nil {
 		stopTimeout = *options.Timeout

diff --git a/pkg/system/process_unix.go b/pkg/system/process_unix.go
@@ -33,6 +33,7 @@ func IsProcessZombie(pid int) (bool, error) {
 	statPath := fmt.Sprintf("/proc/%d/stat", pid)
 	dataBytes, err := os.ReadFile(statPath)
 	if err != nil {
+		// TODO(thaJeztah) should we ignore os.IsNotExist() here? ("/proc/<pid>/stat" will be gone if the process exited)
 		return false, err
 	}
 	data := string(dataBytes)