From d55998be81973be5b083eed56b223c8ce98ce073 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 22 Nov 2013 14:09:37 -0800 Subject: [PATCH 1/3] Remove goroutine leak. Make sure termcap are reset each time. --- commands.go | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/commands.go b/commands.go index 74cbe74600..78f8154a36 100644 --- a/commands.go +++ b/commands.go @@ -2053,9 +2053,18 @@ func (cli *DockerCli) CmdRun(args ...string) error { }() } - // We need to make the chan because the select needs to have a closing - // chan, it can't be uninitialized - hijacked := make(chan bool) + // We need to instanciate the chan because the select needs it. It can + // be closed but can't be uninitialized. + hijacked := make(chan io.Closer) + + // Block the return until the chan gets closed + defer func() { + utils.Debugf("End of CmdRun(), Waiting for hijack to finish.") + if _, ok := <-hijacked; ok { + utils.Errorf("Hijack did not finish (chan still open)") + } + }() + if config.AttachStdin || config.AttachStdout || config.AttachStderr { var ( out, stderr io.Writer @@ -2090,7 +2099,12 @@ func (cli *DockerCli) CmdRun(args ...string) error { // Acknowledge the hijack before starting select { - case <-hijacked: + case closer := <-hijacked: + // Make sure that hijack gets closed when returning. (result + // in closing hijack chan and freeing server's goroutines. + if closer != nil { + defer closer.Close() + } case err := <-errCh: if err != nil { utils.Debugf("Error hijack: %s", err) @@ -2339,7 +2353,12 @@ func (cli *DockerCli) stream(method, path string, in io.Reader, out io.Writer, h return nil } -func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.ReadCloser, stdout, stderr io.Writer, started chan bool) error { +func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.ReadCloser, stdout, stderr io.Writer, started chan io.Closer) error { + defer func() { + if started != nil { + close(started) + } + }() // fixme: refactor client to support redirect re := regexp.MustCompile("/+") path = re.ReplaceAllString(path, "/") @@ -2369,7 +2388,7 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea defer rwc.Close() if started != nil { - started <- true + started <- rwc } var receiveStdout chan error From 1c8ae47770b423b9c2f1125bd25cef0482a4600d Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 22 Nov 2013 14:33:25 -0800 Subject: [PATCH 2/3] Make a validation on links name --- commands.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/commands.go b/commands.go index 78f8154a36..75ad68fea2 100644 --- a/commands.go +++ b/commands.go @@ -1656,6 +1656,18 @@ func (opts AttachOpts) Get(val string) bool { return false } +type LinkOpts []string + +func (link LinkOpts) String() string { + return fmt.Sprintf("%v", []string(link)) +} +func (link LinkOpts) Set(val string) error { + if _, err := parseLink(val); err != nil { + return err + } + return nil +} + // PathOpts stores a unique set of absolute paths type PathOpts map[string]struct{} @@ -1734,6 +1746,7 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co // FIXME: use utils.ListOpts for attach and volumes? flAttach = NewAttachOpts() flVolumes = NewPathOpts() + flLinks = LinkOpts{} flPublish utils.ListOpts flExpose utils.ListOpts @@ -1741,7 +1754,6 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co flDns utils.ListOpts flVolumesFrom utils.ListOpts flLxcOpts utils.ListOpts - flLinks utils.ListOpts flAutoRemove = cmd.Bool("rm", false, "Automatically remove the container when it exits (incompatible with -d)") flDetach = cmd.Bool("d", false, "Detached mode: Run container in the background, print new container id") From c67f9b671df024d06b0af0a0eaa02058e69ee9f5 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 22 Nov 2013 14:42:30 -0800 Subject: [PATCH 3/3] Remove useless New*Opt functions, singleline Opt types --- buildfile.go | 2 +- commands.go | 43 ++++++++++--------------------------------- 2 files changed, 11 insertions(+), 34 deletions(-) diff --git a/buildfile.go b/buildfile.go index dbcec51889..ce157302f6 100644 --- a/buildfile.go +++ b/buildfile.go @@ -241,7 +241,7 @@ func (b *buildFile) CmdVolume(args string) error { volume = []string{args} } if b.config.Volumes == nil { - b.config.Volumes = NewPathOpts() + b.config.Volumes = PathOpts{} } for _, v := range volume { b.config.Volumes[v] = struct{}{} diff --git a/commands.go b/commands.go index 75ad68fea2..a83fefb9ce 100644 --- a/commands.go +++ b/commands.go @@ -1632,15 +1632,7 @@ type ports []int // AttachOpts stores arguments to 'docker run -a', eg. which streams to attach to type AttachOpts map[string]bool -func NewAttachOpts() AttachOpts { - return make(AttachOpts) -} - -func (opts AttachOpts) String() string { - // Cast to underlying map type to avoid infinite recursion - return fmt.Sprintf("%v", map[string]bool(opts)) -} - +func (opts AttachOpts) String() string { return fmt.Sprintf("%v", map[string]bool(opts)) } func (opts AttachOpts) Set(val string) error { if val != "stdin" && val != "stdout" && val != "stderr" { return fmt.Errorf("Unsupported stream name: %s", val) @@ -1649,18 +1641,10 @@ func (opts AttachOpts) Set(val string) error { return nil } -func (opts AttachOpts) Get(val string) bool { - if res, exists := opts[val]; exists { - return res - } - return false -} - +// LinkOpts stores arguments to `docker run -link` type LinkOpts []string -func (link LinkOpts) String() string { - return fmt.Sprintf("%v", []string(link)) -} +func (link LinkOpts) String() string { return fmt.Sprintf("%v", []string(link)) } func (link LinkOpts) Set(val string) error { if _, err := parseLink(val); err != nil { return err @@ -1671,14 +1655,7 @@ func (link LinkOpts) Set(val string) error { // PathOpts stores a unique set of absolute paths type PathOpts map[string]struct{} -func NewPathOpts() PathOpts { - return make(PathOpts) -} - -func (opts PathOpts) String() string { - return fmt.Sprintf("%v", map[string]struct{}(opts)) -} - +func (opts PathOpts) String() string { return fmt.Sprintf("%v", map[string]struct{}(opts)) } func (opts PathOpts) Set(val string) error { var containerPath string @@ -1744,8 +1721,8 @@ func ParseRun(args []string, capabilities *Capabilities) (*Config, *HostConfig, func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Config, *HostConfig, *flag.FlagSet, error) { var ( // FIXME: use utils.ListOpts for attach and volumes? - flAttach = NewAttachOpts() - flVolumes = NewPathOpts() + flAttach = AttachOpts{} + flVolumes = PathOpts{} flLinks = LinkOpts{} flPublish utils.ListOpts @@ -1777,6 +1754,7 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co cmd.Var(flAttach, "a", "Attach to stdin, stdout or stderr.") cmd.Var(flVolumes, "v", "Bind mount a volume (e.g. from the host: -v /host:/container, from docker: -v /container)") + cmd.Var(flLinks, "link", "Add link to another container (name:alias)") cmd.Var(&flPublish, "p", fmt.Sprintf("Publish a container's port to the host (format: %s) (use 'docker port' to see the actual mapping)", PortSpecTemplateFormat)) cmd.Var(&flExpose, "expose", "Expose a port from the container without publishing it to your host") @@ -1784,7 +1762,6 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co cmd.Var(&flDns, "dns", "Set custom dns servers") cmd.Var(&flVolumesFrom, "volumes-from", "Mount volumes from the specified container(s)") cmd.Var(&flLxcOpts, "lxc-conf", "Add custom lxc options -lxc-conf=\"lxc.cgroup.cpuset.cpus = 0,1\"") - cmd.Var(&flLinks, "link", "Add link to another container (name:alias)") if err := cmd.Parse(args); err != nil { return nil, nil, cmd, err @@ -1910,9 +1887,9 @@ func parseRun(cmd *flag.FlagSet, args []string, capabilities *Capabilities) (*Co OpenStdin: *flStdin, Memory: flMemory, CpuShares: *flCpuShares, - AttachStdin: flAttach.Get("stdin"), - AttachStdout: flAttach.Get("stdout"), - AttachStderr: flAttach.Get("stderr"), + AttachStdin: flAttach["stdin"], + AttachStdout: flAttach["stdout"], + AttachStderr: flAttach["stderr"], Env: envs, Cmd: runCmd, Dns: flDns,