Skip to content

Commit 2c17edf

Browse files
committed
cli/connhelper/commandcon.New: pass context with WithoutCancel
Passing the context to the constructor, but explicitly making it non-cancelable and add a comment describing why context-cancelation should not be propagated. Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent e6ee7ea commit 2c17edf

File tree

1 file changed

+16
-6
lines changed

1 file changed

+16
-6
lines changed

cli/connhelper/commandconn/commandconn.go

+16-6
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,28 @@ import (
3333
)
3434

3535
// New returns net.Conn
36-
func New(_ context.Context, cmd string, args ...string) (net.Conn, error) {
37-
var (
38-
c commandConn
39-
err error
40-
)
41-
c.cmd = exec.Command(cmd, args...)
36+
func New(ctx context.Context, cmd string, args ...string) (net.Conn, error) {
37+
// Don't kill the ssh process if the context is cancelled. Killing the
38+
// ssh process causes an error when go's http.Client tries to reuse the
39+
// net.Conn (commandConn).
40+
//
41+
// Not passing down the Context might seem counter-intuitive, but in this
42+
// case, the lifetime of the process should be managed by the http.Client,
43+
// not the caller's Context.
44+
//
45+
// Further details;;
46+
//
47+
// - https://github.com/docker/cli/pull/3900
48+
// - https://github.com/docker/compose/issues/9448#issuecomment-1264263721
49+
ctx = context.WithoutCancel(ctx)
50+
c := commandConn{cmd: exec.CommandContext(ctx, cmd, args...)}
4251
// we assume that args never contains sensitive information
4352
logrus.Debugf("commandconn: starting %s with %v", cmd, args)
4453
c.cmd.Env = os.Environ()
4554
c.cmd.SysProcAttr = &syscall.SysProcAttr{}
4655
setPdeathsig(c.cmd)
4756
createSession(c.cmd)
57+
var err error
4858
c.stdin, err = c.cmd.StdinPipe()
4959
if err != nil {
5060
return nil, err

0 commit comments

Comments
 (0)