Переглянути джерело

daemon: killWithSignal, killPossiblyDeadProcess: accept syscall.Signal

This helps reducing some type-juggling / conversions further up
the stack.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn 3 роки тому
батько
коміт
ea1eb449b7
4 змінених файлів з 24 додано та 20 видалено
  1. 8 6
      daemon/container_operations_unix.go
  2. 12 12
      daemon/kill.go
  3. 3 2
      daemon/stop.go
  4. 1 0
      pkg/system/process_unix.go

+ 8 - 6
daemon/container_operations_unix.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"os"
 	"path/filepath"
 	"path/filepath"
 	"strconv"
 	"strconv"
+	"syscall"
 
 
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/daemon/links"
 	"github.com/docker/docker/daemon/links"
@@ -336,24 +337,25 @@ func (daemon *Daemon) cleanupSecretDir(c *container.Container) {
 
 
 func killProcessDirectly(container *container.Container) error {
 func killProcessDirectly(container *container.Container) error {
 	pid := container.GetPID()
 	pid := container.GetPID()
-	// Ensure that we don't kill ourselves
 	if pid == 0 {
 	if pid == 0 {
+		// Ensure that we don't kill ourselves
 		return nil
 		return nil
 	}
 	}
 
 
-	if err := unix.Kill(pid, 9); err != nil {
+	if err := unix.Kill(pid, syscall.SIGKILL); err != nil {
 		if err != unix.ESRCH {
 		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)
 	// In case there were some exceptions(e.g., state of zombie and D)
 	if system.IsProcessAlive(pid) {
 	if system.IsProcessAlive(pid) {
 		// Since we can not kill a zombie pid, add zombie check here
 		// Since we can not kill a zombie pid, add zombie check here
 		isZombie, err := system.IsProcessZombie(pid)
 		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 {
 		if err != nil {
 			logrus.WithError(err).WithField("container", container.ID).Warn("Container state is invalid")
 			logrus.WithError(err).WithField("container", container.ID).Warn("Container state is invalid")
 			return err
 			return err

+ 12 - 12
daemon/kill.go

@@ -17,7 +17,7 @@ import (
 
 
 type errNoSuchProcess struct {
 type errNoSuchProcess struct {
 	pid    int
 	pid    int
-	signal int
+	signal syscall.Signal
 }
 }
 
 
 func (e errNoSuchProcess) Error() string {
 func (e errNoSuchProcess) Error() string {
@@ -37,21 +37,22 @@ func isErrNoSuchProcess(err error) bool {
 // If no signal is given (sig 0), then Kill with SIGKILL and wait
 // If no signal is given (sig 0), then Kill with SIGKILL and wait
 // for the container to exit.
 // for the container to exit.
 // If a signal is given, then just send it to the container and return.
 // 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)
 	container, err := daemon.GetContainer(name)
 	if err != nil {
 	if err != nil {
 		return err
 		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)
 		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 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.Kill(container)
 	}
 	}
-	return daemon.killWithSignal(container, int(sig))
+	return daemon.killWithSignal(container, sig)
 }
 }
 
 
 // killWithSignal sends the container the given signal. This wrapper for the
 // killWithSignal sends the container the given signal. This wrapper for the
@@ -59,8 +60,7 @@ func (daemon *Daemon) ContainerKill(name string, sig uint64) error {
 // to send the signal. An error is returned if the container is paused
 // 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
 // or not running, or if there is a problem returned from the
 // underlying kill command.
 // 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)
 	logrus.Debugf("Sending kill signal %d to container %s", stopSignal, container.ID)
 	container.Lock()
 	container.Lock()
 	defer container.Unlock()
 	defer container.Unlock()
@@ -140,7 +140,7 @@ func (daemon *Daemon) Kill(container *containerpkg.Container) error {
 	}
 	}
 
 
 	// 1. Send SIGKILL
 	// 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.
 		// kill failed, check if process is no longer running.
 		if isErrNoSuchProcess(err) {
 		if isErrNoSuchProcess(err) {
 			return nil
 			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.
 // 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)
 	err := daemon.killWithSignal(container, sig)
 	if errdefs.IsNotFound(err) {
 	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
 	return err
 }
 }

+ 3 - 2
daemon/stop.go

@@ -2,6 +2,7 @@ package daemon // import "github.com/docker/docker/daemon"
 
 
 import (
 import (
 	"context"
 	"context"
+	"syscall"
 	"time"
 	"time"
 
 
 	containertypes "github.com/docker/docker/api/types/container"
 	containertypes "github.com/docker/docker/api/types/container"
@@ -42,7 +43,7 @@ func (daemon *Daemon) containerStop(ctx context.Context, ctr *container.Containe
 	}
 	}
 
 
 	var (
 	var (
-		stopSignal  = ctr.StopSignal()
+		stopSignal  = syscall.Signal(ctr.StopSignal())
 		stopTimeout = ctr.StopTimeout()
 		stopTimeout = ctr.StopTimeout()
 	)
 	)
 	if options.Signal != "" {
 	if options.Signal != "" {
@@ -50,7 +51,7 @@ func (daemon *Daemon) containerStop(ctx context.Context, ctr *container.Containe
 		if err != nil {
 		if err != nil {
 			return errdefs.InvalidParameter(err)
 			return errdefs.InvalidParameter(err)
 		}
 		}
-		stopSignal = int(sig)
+		stopSignal = sig
 	}
 	}
 	if options.Timeout != nil {
 	if options.Timeout != nil {
 		stopTimeout = *options.Timeout
 		stopTimeout = *options.Timeout

+ 1 - 0
pkg/system/process_unix.go

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