diff --git a/service/gcs/core/gcs/gcs.go b/service/gcs/core/gcs/gcs.go index e1ce908f..84b5ebe6 100644 --- a/service/gcs/core/gcs/gcs.go +++ b/service/gcs/core/gcs/gcs.go @@ -118,7 +118,7 @@ func (e *containerCacheEntry) RemoveMappedDirectory(dir prot.MappedDirectory) { type processCacheEntry struct { ExitStatus oslayer.ProcessExitState ExitHooks []func(oslayer.ProcessExitState) - Master *os.File + Tty *stdio.TtyRelay } func newProcessCacheEntry() *processCacheEntry { @@ -219,7 +219,7 @@ func (c *gcsCore) ExecProcess(id string, params prot.ProcessParameters, stdioSet containerEntry.container = container p = container - processEntry.Master = p.Console() + processEntry.Tty = p.Tty() // Configure network adapters in the namespace. for _, adapter := range containerEntry.NetworkAdapters { @@ -267,7 +267,7 @@ func (c *gcsCore) ExecProcess(id string, params prot.ProcessParameters, stdioSet if err != nil { return -1, err } - processEntry.Master = p.Console() + processEntry.Tty = p.Tty() go func() { state, err := p.Wait() @@ -403,10 +403,10 @@ func (c *gcsCore) RunExternalProcess(params prot.ProcessParameters, stdioSet *st cmd.SetEnv(ociProcess.Env) var relay *stdio.TtyRelay - var master *os.File if params.EmulateConsole { // Allocate a console for the process. var ( + master *os.File consolePath string ) master, consolePath, err = stdio.NewConsole() @@ -449,7 +449,7 @@ func (c *gcsCore) RunExternalProcess(params prot.ProcessParameters, stdioSet *st } processEntry := newProcessCacheEntry() - processEntry.Master = master + processEntry.Tty = relay go func() { if err := cmd.Wait(); err != nil { // TODO: When cmd is a shell, and last command in the shell @@ -629,11 +629,11 @@ func (c *gcsCore) ResizeConsole(id string, pid int, height, width uint16) error c.containerCacheMutex.Unlock() } - if err := stdio.ResizeConsole(p.Master, height, width); err != nil { - return errors.Wrapf(err, "failed to resize console for pid: %d", pid) + if p.Tty == nil { + return fmt.Errorf("pid: %d, is not a tty and cannot be resized", pid) } - return nil + return p.Tty.ResizeConsole(height, width) } // setupMappedVirtualDisks is a helper function which calls into the functions diff --git a/service/gcs/runtime/mockruntime/mockruntime.go b/service/gcs/runtime/mockruntime/mockruntime.go index a4170bc5..7d2f8a6f 100644 --- a/service/gcs/runtime/mockruntime/mockruntime.go +++ b/service/gcs/runtime/mockruntime/mockruntime.go @@ -2,7 +2,6 @@ package mockruntime import ( - "os" "sync" "github.com/Microsoft/opengcs/service/gcs/oslayer" @@ -47,7 +46,7 @@ func (c *container) Pid() int { return 101 } -func (c *container) Console() *os.File { +func (c *container) Tty() *stdio.TtyRelay { return nil } diff --git a/service/gcs/runtime/runc/runc.go b/service/gcs/runtime/runc/runc.go index 2aa6d08e..b73834e2 100644 --- a/service/gcs/runtime/runc/runc.go +++ b/service/gcs/runtime/runc/runc.go @@ -48,23 +48,22 @@ func (c *container) Pid() int { return c.init.Pid() } -func (c *container) Console() *os.File { - return c.init.master +func (c *container) Tty() *stdio.TtyRelay { + return c.init.relay } type process struct { - c *container - pid int - relay *stdio.TtyRelay - master *os.File + c *container + pid int + relay *stdio.TtyRelay } func (p *process) Pid() int { return p.pid } -func (p *process) Console() *os.File { - return p.master +func (p *process) Tty() *stdio.TtyRelay { + return p.relay } // NewRuntime instantiates a new runcRuntime struct. @@ -570,5 +569,5 @@ func (c *container) startProcess(tempProcessDir string, hasTerminal bool, stdioS if relay != nil { relay.Start() } - return &process{c: c, pid: pid, relay: relay, master: master}, nil + return &process{c: c, pid: pid, relay: relay}, nil } diff --git a/service/gcs/runtime/runtime.go b/service/gcs/runtime/runtime.go index 0ff7501d..13cb9cff 100644 --- a/service/gcs/runtime/runtime.go +++ b/service/gcs/runtime/runtime.go @@ -4,7 +4,6 @@ package runtime import ( "io" - "os" "github.com/Microsoft/opengcs/service/gcs/oslayer" "github.com/Microsoft/opengcs/service/gcs/stdio" @@ -44,7 +43,7 @@ type Process interface { Wait() (oslayer.ProcessExitState, error) Pid() int Delete() error - Console() *os.File + Tty() *stdio.TtyRelay } // Container is an interface to manipulate container state. diff --git a/service/gcs/stdio/stdio.go b/service/gcs/stdio/stdio.go index b71c741e..3cf8901c 100644 --- a/service/gcs/stdio/stdio.go +++ b/service/gcs/stdio/stdio.go @@ -4,10 +4,15 @@ import ( "io" "os" "sync" + "sync/atomic" + "syscall" + "unsafe" "github.com/Microsoft/opengcs/service/gcs/transport" "github.com/Sirupsen/logrus" "github.com/pkg/errors" + + "golang.org/x/sys/unix" ) // ConnectionSet is a structure defining the readers and writers the Core @@ -100,15 +105,37 @@ func (s *ConnectionSet) Files() (_ *FileSet, err error) { } // NewTtyRelay returns a new TTY relay for a given master PTY file. -func (s *ConnectionSet) NewTtyRelay(pty io.ReadWriteCloser) *TtyRelay { +func (s *ConnectionSet) NewTtyRelay(pty *os.File) *TtyRelay { return &TtyRelay{s: s, pty: pty} } // TtyRelay relays IO between a set of stdio connections and a master PTY file. type TtyRelay struct { - wg sync.WaitGroup - s *ConnectionSet - pty io.ReadWriteCloser + closing int32 + wg sync.WaitGroup + s *ConnectionSet + pty *os.File +} + +// ResizeConsole sends the appropriate resize to a pTTY FD +func (r *TtyRelay) ResizeConsole(height, width uint16) error { + type consoleSize struct { + Height uint16 + Width uint16 + x uint16 + y uint16 + } + + r.wg.Add(1) + defer r.wg.Done() + if atomic.LoadInt32(&r.closing) != 0 { + return errors.New("error resizing console pty is closed") + } + + if _, _, err := syscall.Syscall(syscall.SYS_IOCTL, r.pty.Fd(), uintptr(unix.TIOCSWINSZ), uintptr(unsafe.Pointer(&consoleSize{Height: height, Width: width}))); err != 0 { + return err + } + return nil } // Start starts the relay operation. The caller must call Wait to wait @@ -151,6 +178,13 @@ func (r *TtyRelay) Wait() { // Wait for all users of stdioSet and master to finish before closing them. r.wg.Wait() + + // Given the expected use of wait we cannot increment closing before calling r.wg.Wait() + // or all calls to ResizeConsole would fail. However, by calling it after there is a very + // small window that ResizeConsole could still get an invalid Fd. We call wait again to + // enusure that no ResizeConsole call came in before actualling closing the pty. + atomic.StoreInt32(&r.closing, 1) + r.wg.Wait() r.pty.Close() r.s.Close() } diff --git a/service/gcs/stdio/tty.go b/service/gcs/stdio/tty.go index 7524de9f..d2cdafd9 100644 --- a/service/gcs/stdio/tty.go +++ b/service/gcs/stdio/tty.go @@ -7,17 +7,8 @@ import ( "unsafe" "github.com/pkg/errors" - - "golang.org/x/sys/unix" ) -type consoleSize struct { - Height uint16 - Width uint16 - x uint16 - y uint16 -} - // NewConsole allocates a new console and returns the File for its master and // path for its slave. func NewConsole() (*os.File, string, error) { @@ -42,15 +33,6 @@ func NewConsole() (*os.File, string, error) { return master, console, nil } -// ResizeConsole sends the appropriate resize to a pTTY FD -func ResizeConsole(master *os.File, height, width uint16) error { - if master == nil { - return errors.New("failed to resize console with null master fd") - } - - return ioctl(master.Fd(), uintptr(unix.TIOCSWINSZ), uintptr(unsafe.Pointer(&consoleSize{Height: height, Width: width}))) -} - func ioctl(fd uintptr, flag, data uintptr) error { if _, _, err := syscall.Syscall(syscall.SYS_IOCTL, fd, flag, data); err != 0 { return err