Synchronizing ResizeConsole

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

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
 }
 

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
 }

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.

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()
 }

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