ronantakizawa/codereview-bench huggingface dataset:
Repository: moby/moby
File: libnetwork/osl/neigh_linux.go (Go)

Code before:
package osl

import (
	"bytes"
	"context"
	"errors"
	"fmt"
	"net"
	"os"

	"github.com/containerd/log"
	"github.com/vishvananda/netlink"
)

// NeighborSearchError indicates that the neighbor is already present
type NeighborSearchError struct {
	ip      net.IP
	mac     net.HardwareAddr
	present bool
}

func (n NeighborSearchError) Error() string {
	return fmt.Sprintf("Search neighbor failed for IP %v, mac %v, present in db:%t", n.ip, n.mac, n.present)
}

type neigh struct {
	dstIP    net.IP
	dstMac   net.HardwareAddr
	linkName string
	linkDst  string
	family   int
}

func (n *Namespace) findNeighbor(dstIP net.IP, dstMac net.HardwareAddr) *neigh {
	n.mu.Lock()
	defer n.mu.Unlock()

	for _, nh := range n.neighbors {
		if nh.dstIP.Equal(dstIP) && bytes.Equal(nh.dstMac, dstMac) {
			return nh
		}
	}

	return nil
}

// DeleteNeighbor deletes neighbor entry from the sandbox.
func (n *Namespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr) error {
	nh := n.findNeighbor(dstIP, dstMac)
	if nh == nil {
		return NeighborSearchError{dstIP, dstMac, false}

Review comment (performance):
This was the only part I had a comment on in the other PR. Non-blocking (from my perspective), but reposting here for others; https://github.com/moby/moby/pull/50081#discussion_r2108869390

> Not a blocker, but was mostly looking here for trade-offs between performance-optimisation (strings.Builder) and discoverability (being able to grep for the (partial) error message); wasn't sure how much this error is in a "hot path".

Code after:
package osl

import (
	"context"
	"errors"
	"fmt"
	"net"
	"os"
	"strings"

	"github.com/containerd/log"
	"github.com/vishvananda/netlink"
)

// NeighborSearchError indicates that the neighbor is already present
type NeighborSearchError struct {
	ip       net.IP
	mac      net.HardwareAddr
	linkName string
	present  bool
}

func (n NeighborSearchError) Error() string {
	var b strings.Builder
	b.WriteString("neighbor entry ")
	if n.present {
		b.WriteString("already exists ")
	} else {
		b.WriteString("not found ")
	}
	b.WriteString("for IP ")
	b.WriteString(n.ip.String())
	b.WriteString(", mac ")
	b.WriteString(n.mac.String())
	if n.linkName != "" {
		b.WriteString(", link ")
		b.WriteString(n.linkName)
	}
	return b.String()
}

// DeleteNeighbor deletes a neighbor entry from the sandbox.
//
// To delete an entry inserted by [AddNeighbor] the caller must provide the same
// parameters used to add it.
func (n *Namespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr, options ...NeighOption) error {
	nlnh, linkName, err := n.nlNeigh(dstIP, dstMac, options...)
	if err != nil {
		return err
	}
Repository: moby/moby
File: profiles/seccomp/default_linux.go (Go)

Code before:
			Includes: &Filter{
				Arches: []string{"riscv64"},
			},
		},
		{
			LinuxSyscall: specs.LinuxSyscall{
				Names: []string{
					"open_by_handle_at",
				},
				Action: specs.ActAllow,
			},
			Includes: &Filter{
				Caps: []string{"CAP_DAC_READ_SEARCH"},
			},
		},
		{
			LinuxSyscall: specs.LinuxSyscall{
				Names: []string{
					"bpf",
					"clone",
					"clone3",
					"fanotify_init",
					"fsconfig",
					"fsmount",
					"fsopen",
					"fspick",
					"lookup_dcookie",
					"mount",
					"mount_setattr",
					"move_mount",
					"open_tree",
					"perf_event_open",
					"quotactl",
					"quotactl_fd",
					"setdomainname",
					"sethostname",
					"setns",
					"syslog",
					"umount",
					"umount2",
					"unshare",
				},
				Action: specs.ActAllow,
			},
			Includes: &Filter{
				Caps: []string{"CAP_SYS_ADMIN"},
			},
		},
		{
			LinuxSyscall: specs.LinuxSyscall{
				Names: []string{

Review comment (suggestion):
Without having given it much thought, and based on names, `get`, and `list` seem potentially safe (but not sure how software handles half of these being blocked, and the other half not blocked).


@AkihiroSuda @tonistiigi for thoughts here.

Code after:
		},
		{
			LinuxSyscall: specs.LinuxSyscall{
				Names: []string{
					"open_by_handle_at",
				},
				Action: specs.ActAllow,
			},
			Includes: &Filter{
				Caps: []string{"CAP_DAC_READ_SEARCH"},
			},
		},
		{
			LinuxSyscall: specs.LinuxSyscall{
				Names: []string{
					"bpf",
					"clone",
					"clone3",
					"fanotify_init",
					"fsconfig",
					"fsmount",
					"fsopen",
					"fspick",
					"lookup_dcookie",
					"lsm_get_self_attr", // kernel v6.8, libseccomp v2.6.0
					"lsm_list_modules",  // kernel v6.8, libseccomp v2.6.0
					"lsm_set_self_attr", // kernel v6.8, libseccomp v2.6.0
					"mount",
					"mount_setattr",
					"move_mount",
					"open_tree",
					"perf_event_open",
					"quotactl",
					"quotactl_fd",
					"setdomainname",
					"sethostname",
					"setns",
					"syslog",
					"umount",
					"umount2",
					"unshare",
				},
				Action: specs.ActAllow,
			},
			Includes: &Filter{
				Caps: []string{"CAP_SYS_ADMIN"},
			},
		},
		{
			LinuxSyscall: specs.LinuxSyscall{
				Names: []string{
Repository: moby/moby
File: client/client_unix.go (Go)

Code before:
//go:build !windows

package client

import (
	"context"
	"net"
	"syscall"
)

// DefaultDockerHost defines OS-specific default host if the DOCKER_HOST
// (EnvOverrideHost) environment variable is unset or empty.
const DefaultDockerHost = "unix:///var/run/docker.sock"

// dialPipeContext connects to a Windows named pipe. It is not supported on non-Windows.
func dialPipeContext(ctx context.Context, string) (net.Conn, error) {
	return nil, syscall.EAFNOSUPPORT
}


Review comment (suggestion):
We could probably change this to `ErrProtocolNotAvailable` (or any other error?) 🤔

Code after:
//go:build !windows

package client

import (
	"context"
	"net"
	"syscall"
)

// DefaultDockerHost defines OS-specific default host if the DOCKER_HOST
// (EnvOverrideHost) environment variable is unset or empty.
const DefaultDockerHost = "unix:///var/run/docker.sock"

// dialPipeContext connects to a Windows named pipe. It is not supported on non-Windows.
func dialPipeContext(_ context.Context, _ string) (net.Conn, error) {
	return nil, syscall.EAFNOSUPPORT
}
Repository: moby/moby
File: client/client.go (Go)

Code before:
		return nil
	}
	return cli.baseTransport.DialContext
}

// Dialer returns a dialer for a raw stream connection, with an HTTP/1.1 header,
// that can be used for proxying the daemon connection. It is used by
// ["docker dial-stdio"].
//
// ["docker dial-stdio"]: https://github.com/docker/cli/pull/1014
func (cli *Client) Dialer() func(context.Context) (net.Conn, error) {
	return cli.dialer()
}

func (cli *Client) dialer() func(context.Context) (net.Conn, error) {
	return func(ctx context.Context) (net.Conn, error) {
		if dialFn := cli.dialerFromTransport(); dialFn != nil {
			return dialFn(ctx, cli.proto, cli.addr)
		}
		switch cli.proto {
		case "unix":
			return net.Dial(cli.proto, cli.addr)
		case "npipe":
			return sockets.DialPipe(cli.addr, 32*time.Second)
		default:
			if tlsConfig := cli.tlsConfig(); tlsConfig != nil {
				return tls.Dial(cli.proto, cli.addr, tlsConfig)
			}
			return net.Dial(cli.proto, cli.addr)
		}
	}
}


Review comment (suggestion):
🫠 looks like `testutil` has its own separate implementation of all of this 
https://github.com/moby/moby/blob/c29de525b8fe0ef82f38306f4b20223c78a72c2d/testutil/request/npipe_windows.go#L10-L12

https://github.com/moby/moby/blob/c29de525b8fe0ef82f38306f4b20223c78a72c2d/testutil/request/request.go#L197-L224



Code after:
		return nil
	}
	return cli.baseTransport.DialContext
}

// Dialer returns a dialer for a raw stream connection, with an HTTP/1.1 header,
// that can be used for proxying the daemon connection. It is used by
// ["docker dial-stdio"].
//
// ["docker dial-stdio"]: https://github.com/docker/cli/pull/1014
func (cli *Client) Dialer() func(context.Context) (net.Conn, error) {
	return cli.dialer()
}

func (cli *Client) dialer() func(context.Context) (net.Conn, error) {
	return func(ctx context.Context) (net.Conn, error) {
		if dialFn := cli.dialerFromTransport(); dialFn != nil {
			return dialFn(ctx, cli.proto, cli.addr)
		}
		switch cli.proto {
		case "unix":
			return net.Dial(cli.proto, cli.addr)
		case "npipe":
			ctx, cancel := context.WithTimeout(ctx, 32*time.Second)
			defer cancel()
			return dialPipeContext(ctx, cli.addr)
		default:
			if tlsConfig := cli.tlsConfig(); tlsConfig != nil {
				return tls.Dial(cli.proto, cli.addr, tlsConfig)
			}
			return net.Dial(cli.proto, cli.addr)
		}
	}
}
Repository: moby/moby
File: api/server/router/image/inspect_response_test.go (Go)

Code before:
package image

import (
	"encoding/json"
	"testing"

	"github.com/docker/docker/api/types/image"
	dockerspec "github.com/moby/docker-image-spec/specs-go/v1"
	"gotest.tools/v3/assert"
	is "gotest.tools/v3/assert/cmp"
)

func TestInspectResponse(t *testing.T) {
	imgInspect := &image.InspectResponse{
		Config: &dockerspec.DockerOCIImageConfig{},
	}
	imgInspect.Config.Cmd = []string{"/bin/sh"}

	out, err := json.Marshal(&inspectResponse{imgInspect})
	assert.NilError(t, err)

	// We're only intereted in the "Config" field
	const expected = `"Config":{"Cmd":["/bin/sh"],"Entrypoint":null,"Env":null,"Labels":null,"OnBuild":null,"User":"","Volumes":null,"WorkingDir":""}`
	assert.Assert(t, is.Contains(string(out), expected))
}


Review comment (suggestion):
```suggestion
	// We're only interested in the "Config" field
```

Code after:
package image

import (
	"encoding/json"
	"testing"

	"github.com/docker/docker/api/types/image"
	dockerspec "github.com/moby/docker-image-spec/specs-go/v1"
	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
	"gotest.tools/v3/assert"
	is "gotest.tools/v3/assert/cmp"
)

func TestInspectResponse(t *testing.T) {
	tests := []struct {
		doc          string
		cfg          *ocispec.ImageConfig
		legacyConfig map[string]any
		expected     string
	}{
		{
			doc:      "empty",
			expected: `null`,
		},
		{
			doc: "no legacy config",
			cfg: &ocispec.ImageConfig{
				Cmd:        []string{"/bin/sh"},
				StopSignal: "SIGQUIT",
			},
			expected: `{"Cmd":["/bin/sh"],"StopSignal":"SIGQUIT"}`,
		},
		{
			doc: "api < v1.50",
			cfg: &ocispec.ImageConfig{
				Cmd:        []string{"/bin/sh"},
				StopSignal: "SIGQUIT",
			},
			legacyConfig: legacyConfigFields["v1.49"],
			expected:     `{"AttachStderr":false,"AttachStdin":false,"AttachStdout":false,"Cmd":["/bin/sh"],"Domainname":"","Entrypoint":null,"Env":null,"Hostname":"","Image":"","Labels":null,"OnBuild":null,"OpenStdin":false,"StdinOnce":false,"StopSignal":"SIGQUIT","Tty":false,"User":"","Volumes":null,"WorkingDir":""}`,
		},
		{
			doc: "api >= v1.50",
			cfg: &ocispec.ImageConfig{
				Cmd:        []string{"/bin/sh"},
				StopSignal: "SIGQUIT",
			},
Repository: moby/moby
File: libnetwork/controller.go (Go)

Code before:
		if s.containerID == id {
			sb = s
			break
		}
	}
	c.mu.Unlock()

	// It is not an error if sandbox is not available
	if sb == nil {
		return nil
	}

	return sb.Delete(ctx)
}

func (c *Controller) loadDriver(networkType string) error {
	var err error

	if pg := c.GetPluginGetter(); pg != nil {
		_, err = pg.Get(networkType, driverapi.NetworkPluginEndpointType, plugingetter.Lookup)
	} else {
		_, err = plugins.Get(networkType, driverapi.NetworkPluginEndpointType)
	}

	if err != nil {
		if errors.Is(errors.Cause(err), plugins.ErrNotFound) {
			return types.NotFoundErrorf("%v", err)
		}
		return err
	}

	return nil
}

func (c *Controller) loadIPAMDriver(name string) error {
	var err error

	if pg := c.GetPluginGetter(); pg != nil {
		_, err = pg.Get(name, ipamapi.PluginEndpointType, plugingetter.Lookup)
	} else {
		_, err = plugins.Get(name, ipamapi.PluginEndpointType)
	}

	if err != nil {
		if errors.Is(errors.Cause(err), plugins.ErrNotFound) {
			return types.NotFoundErrorf("%v", err)
		}
		return err
	}

	return nil

Review comment (suggestion):
We should probably remove the `errors.Cause()` here; looking at the old code, the `errors.Cause()` was added for cases where the error was wrapped through `pkg/errors` - newer versions of `pkg/errors` also implement `Unwrap`, so the `pkg/errors`-specific handling (using the `Causer` interface) probably became redundant, and just using `errors.Is` should work

```suggestion
		if errors.Is(err, plugins.ErrNotFound) {
```

See https://go.dev/play/p/t6N7AVn-FL2


```go
package main

import (
	"fmt"

	"github.com/pkg/errors"
)

func main() {
	wrappedErr := fmt.Errorf("hello error")

	err := errors.Wrapf(wrappedErr, "wrapped")

	if errors.Cause(err) == wrappedErr {
		fmt.Println("it's a cause")
	}

	if errors.Is(err, wrappedErr) {
		fmt.Println("it's a wrap")
	}

	if errors.Is(errors.Cause(err), wrappedErr) {
		fmt.Println("causer and errors.Is")
	}

}
```



Code after:
		if s.containerID == id {
			sb = s
			break
		}
	}
	c.mu.Unlock()

	// It is not an error if sandbox is not available
	if sb == nil {
		return nil
	}

	return sb.Delete(ctx)
}

func (c *Controller) loadDriver(networkType string) error {
	var err error

	if pg := c.GetPluginGetter(); pg != nil {
		_, err = pg.Get(networkType, driverapi.NetworkPluginEndpointType, plugingetter.Lookup)
	} else {
		_, err = plugins.Get(networkType, driverapi.NetworkPluginEndpointType)
	}

	if err != nil {
		if errors.Is(err, plugins.ErrNotFound) {
			return types.NotFoundErrorf("%v", err)
		}
		return err
	}

	return nil
}

func (c *Controller) loadIPAMDriver(name string) error {
	var err error

	if pg := c.GetPluginGetter(); pg != nil {
		_, err = pg.Get(name, ipamapi.PluginEndpointType, plugingetter.Lookup)
	} else {
		_, err = plugins.Get(name, ipamapi.PluginEndpointType)
	}

	if err != nil {
		if errors.Is(err, plugins.ErrNotFound) {
			return types.NotFoundErrorf("%v", err)
		}
		return err
	}

	return nil
Repository: moby/moby
File: libnetwork/controller.go (Go)

Code before:
		_, err = pg.Get(networkType, driverapi.NetworkPluginEndpointType, plugingetter.Lookup)
	} else {
		_, err = plugins.Get(networkType, driverapi.NetworkPluginEndpointType)
	}

	if err != nil {
		if errors.Is(errors.Cause(err), plugins.ErrNotFound) {
			return types.NotFoundErrorf("%v", err)
		}
		return err
	}

	return nil
}

func (c *Controller) loadIPAMDriver(name string) error {
	var err error

	if pg := c.GetPluginGetter(); pg != nil {
		_, err = pg.Get(name, ipamapi.PluginEndpointType, plugingetter.Lookup)
	} else {
		_, err = plugins.Get(name, ipamapi.PluginEndpointType)
	}

	if err != nil {
		if errors.Is(errors.Cause(err), plugins.ErrNotFound) {
			return types.NotFoundErrorf("%v", err)
		}
		return err
	}

	return nil
}

func (c *Controller) getIPAMDriver(name string) (ipamapi.Ipam, *ipamapi.Capability, error) {
	id, caps := c.ipamRegistry.IPAM(name)
	if id == nil {
		// Might be a plugin name. Try loading it
		if err := c.loadIPAMDriver(name); err != nil {
			return nil, nil, err
		}

		// Now that we resolved the plugin, try again looking up the registry
		id, caps = c.ipamRegistry.IPAM(name)
		if id == nil {
			return nil, nil, types.InvalidParameterErrorf("invalid ipam driver: %q", name)
		}
	}

	return id, caps, nil
}

Review comment (suggestion):
Same here, and other places where we call `errors.Cause` first.

Code after:
		_, err = pg.Get(networkType, driverapi.NetworkPluginEndpointType, plugingetter.Lookup)
	} else {
		_, err = plugins.Get(networkType, driverapi.NetworkPluginEndpointType)
	}

	if err != nil {
		if errors.Is(err, plugins.ErrNotFound) {
			return types.NotFoundErrorf("%v", err)
		}
		return err
	}

	return nil
}

func (c *Controller) loadIPAMDriver(name string) error {
	var err error

	if pg := c.GetPluginGetter(); pg != nil {
		_, err = pg.Get(name, ipamapi.PluginEndpointType, plugingetter.Lookup)
	} else {
		_, err = plugins.Get(name, ipamapi.PluginEndpointType)
	}

	if err != nil {
		if errors.Is(err, plugins.ErrNotFound) {
			return types.NotFoundErrorf("%v", err)
		}
		return err
	}

	return nil
}

func (c *Controller) getIPAMDriver(name string) (ipamapi.Ipam, *ipamapi.Capability, error) {
	id, caps := c.ipamRegistry.IPAM(name)
	if id == nil {
		// Might be a plugin name. Try loading it
		if err := c.loadIPAMDriver(name); err != nil {
			return nil, nil, err
		}

		// Now that we resolved the plugin, try again looking up the registry
		id, caps = c.ipamRegistry.IPAM(name)
		if id == nil {
			return nil, nil, types.InvalidParameterErrorf("invalid ipam driver: %q", name)
		}
	}

	return id, caps, nil
}
Repository: moby/moby
File: errdefs/helpers_test.go (Go)

Code before:
package errdefs

import (
	"errors"
	"fmt"
	"testing"

	cerrdefs "github.com/containerd/errdefs"
)

var errTest = errors.New("this is a test")

type wrapped interface {
	Unwrap() error
}

func TestNotFound(t *testing.T) {
	if cerrdefs.IsNotFound(errTest) {
		t.Fatalf("did not expect not found error, got %T", errTest)
	}
	e := NotFound(errTest)
	if !cerrdefs.IsNotFound(e) {
		t.Fatalf("expected not found error, got: %T", e)
	}
	if cause := e.(wrapped).Unwrap(); cause != errTest { //nolint:errorlint
		t.Fatalf("causual should be errTest, got: %v", cause)
	}
	if !errors.Is(e, errTest) {
		t.Fatalf("expected not found error to match errTest")
	}

	wrapped := fmt.Errorf("foo: %w", e)
	if !cerrdefs.IsNotFound(wrapped) {
		t.Fatalf("expected not found error, got: %T", wrapped)
	}
}

func TestConflict(t *testing.T) {
	if cerrdefs.IsConflict(errTest) {
		t.Fatalf("did not expect conflict error, got %T", errTest)
	}
	e := Conflict(errTest)
	if !cerrdefs.IsConflict(e) {
		t.Fatalf("expected conflict error, got: %T", e)
	}
	if cause := e.(wrapped).Unwrap(); cause != errTest { //nolint:errorlint
		t.Fatalf("causual should be errTest, got: %v", cause)
	}
	if !errors.Is(e, errTest) {
		t.Fatalf("expected conflict error to match errTest")

Review comment (bug):
🙈 The `//nolint:errorlint` that was added will very likely fail when you come to work on the `whyNoLint` rule for go-critic in https://github.com/moby/moby/pull/50171

Probably best to fix it straight-away, otherwise we end up updating these lines again. Could be something like;

```suggestion
	if cause := e.(wrapped).Unwrap(); cause != errTest { //nolint:errorlint // not using errors.Is, as this tests for the unwrapped error.
```

Code after:
package errdefs

import (
	"errors"
	"fmt"
	"testing"

	cerrdefs "github.com/containerd/errdefs"
)

var errTest = errors.New("this is a test")

type wrapped interface {
	Unwrap() error
}

func TestNotFound(t *testing.T) {
	if cerrdefs.IsNotFound(errTest) {
		t.Fatalf("did not expect not found error, got %T", errTest)
	}
	e := NotFound(errTest)
	if !cerrdefs.IsNotFound(e) {
		t.Fatalf("expected not found error, got: %T", e)
	}
	if cause := e.(wrapped).Unwrap(); cause != errTest { //nolint:errorlint // not using errors.Is, as this tests for the unwrapped error.
		t.Fatalf("causual should be errTest, got: %v", cause)
	}
	if !errors.Is(e, errTest) {
		t.Fatalf("expected not found error to match errTest")
	}

	wrapped := fmt.Errorf("foo: %w", e)
	if !cerrdefs.IsNotFound(wrapped) {
		t.Fatalf("expected not found error, got: %T", wrapped)
	}
}

func TestConflict(t *testing.T) {
	if cerrdefs.IsConflict(errTest) {
		t.Fatalf("did not expect conflict error, got %T", errTest)
	}
	e := Conflict(errTest)
	if !cerrdefs.IsConflict(e) {
		t.Fatalf("expected conflict error, got: %T", e)
	}
	if cause := e.(wrapped).Unwrap(); cause != errTest { //nolint:errorlint // not using errors.Is, as this tests for the unwrapped error.
		t.Fatalf("causual should be errTest, got: %v", cause)
	}
	if !errors.Is(e, errTest) {
		t.Fatalf("expected conflict error to match errTest")
Repository: moby/moby
File: daemon/stop.go (Go)

Code before:
	// Cancelling the request should not cancel the stop.
	ctx = context.WithoutCancel(ctx)

	if !ctr.IsRunning() {
		return nil
	}

	var (
		stopSignal  = ctr.StopSignal()
		stopTimeout = ctr.StopTimeout()
	)
	if options.Signal != "" {
		sig, err := signal.ParseSignal(options.Signal)
		if err != nil {
			return errdefs.InvalidParameter(err)
		}
		stopSignal = sig
	}
	if options.Timeout != nil {
		stopTimeout = *options.Timeout
	}

	var wait time.Duration
	if stopTimeout >= 0 {
		wait = time.Duration(stopTimeout) * time.Second
	}

	// 1. Send a stop signal
	err := daemon.killPossiblyDeadProcess(ctr, stopSignal)
	if err != nil {
		wait = 2 * time.Second
	}

	var subCtx context.Context
	var cancel context.CancelFunc
	if stopTimeout >= 0 {
		subCtx, cancel = context.WithTimeout(ctx, wait)
	} else {
		subCtx, cancel = context.WithCancel(ctx)
	}
	defer cancel()

	if status := <-ctr.Wait(subCtx, containertypes.WaitConditionNotRunning); status.Err() == nil {
		daemon.LogContainerEvent(ctr, events.ActionStop)
		// Ensure container status changes are committed by handler of container exit before returning control to the caller
		ctr.Lock()
		defer ctr.Unlock()
		// container did exit, so ignore any previous errors and return
		return nil
	}


Review comment (suggestion):
Considering that the "happy path" also ends with the mutex lock&unlock, perhaps it would make more sense to put the mutex lock&unlock into the defer too?

```suggestion
	defer func() {
		if retErr == nil {
			daemon.LogContainerEvent(ctr, events.ActionStop)
			
			// Ensure container status changes are committed by handler of container exit before returning control to the caller
			ctr.Lock()
			defer ctr.Unlock()
		}
	}()
```



Code after:
	// Cancelling the request should not cancel the stop.
	ctx = context.WithoutCancel(ctx)

	if !ctr.IsRunning() {
		return nil
	}

	var (
		stopSignal  = ctr.StopSignal()
		stopTimeout = ctr.StopTimeout()
	)
	if options.Signal != "" {
		sig, err := signal.ParseSignal(options.Signal)
		if err != nil {
			return errdefs.InvalidParameter(err)
		}
		stopSignal = sig
	}
	if options.Timeout != nil {
		stopTimeout = *options.Timeout
	}

	var wait time.Duration
	if stopTimeout >= 0 {
		wait = time.Duration(stopTimeout) * time.Second
	}
	defer func() {
		if retErr == nil {
			daemon.LogContainerEvent(ctr, events.ActionStop)
			// Ensure container status changes are committed by handler of container exit before returning control to the caller
			ctr.Lock()
			defer ctr.Unlock()
		}
	}()

	// 1. Send a stop signal
	err := daemon.killPossiblyDeadProcess(ctr, stopSignal)
	if err != nil {
		wait = 2 * time.Second
	}

	var subCtx context.Context
	var cancel context.CancelFunc
	if stopTimeout >= 0 {
		subCtx, cancel = context.WithTimeout(ctx, wait)
	} else {
		subCtx, cancel = context.WithCancel(ctx)
	}
	defer cancel()

	if status := <-ctr.Wait(subCtx, containertypes.WaitConditionNotRunning); status.Err() == nil {