summaryrefslogtreecommitdiff
path: root/cmd
diff options
context:
space:
mode:
authorW. Trevor King <wking@tremily.us>2018-06-28 08:00:23 -0700
committerAtomic Bot <atomic-devel@projectatomic.io>2018-06-28 15:53:02 +0000
commitc82166afabd63f6271990be89ffa6609d3dc6712 (patch)
treecaa892a71ef31efd282ee677605e6f089e33f5de /cmd
parent333ab8c21120c3f7e6b3df7a6a49ca860f079266 (diff)
downloadpodman-c82166afabd63f6271990be89ffa6609d3dc6712.tar.gz
podman-c82166afabd63f6271990be89ffa6609d3dc6712.tar.bz2
podman-c82166afabd63f6271990be89ffa6609d3dc6712.zip
cmd/podman/utils.go: Cancel-able resize writes
On Thu, Jun 28, 2018 at 03:48:26AM -0700, Marco Vedovati wrote [1]: > The root cause is a deadlock between two channel writes made by two > different goroutines: > > 1. `resizeTty() : go func(){} : sendUpdate()` is sending a resize > message thru `resize` right at the beginning, but the channel is > never read if some startup error occurs. > > 2. Upon program termination, `startAttachCtr() : defer func(){} ` is > telling the goroutine in "1." to stop via the `resizeTerminate` > channel. But that guy is still waiting for the write to `resize` > to complete so the the termination message is never read. > > I think the go deadlock detection does not kick in because not all > goroutines are seen as asleep. E.g. `os/signal Notify()` is enough > to have the deadlock not detected. 333ab8c2 (Fix podman hangs when detecting startup error in container attached mode, 2018-06-27, #1010) addressed this with a deferred drain. This commit adjusts that approach to use a single select to cover "have we been canceled?", "has there been a resize signal?", and (when we have one) "can we write the most recent resize event to the resize channel?". A side benefit to this approach is that if we have a slow resize consumer and several resize signals, the resizeTty function will keep updating its local resizeEvent. Once the resize channel is able to accept, only the most-recent event will be written. Previously we'd have written one resize event for every received signal, even if the resize consumer was falling behind. [1]: https://github.com/projectatomic/libpod/pull/1010#issuecomment-400994436 Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #1018 Approved by: rhatdan
Diffstat (limited to 'cmd')
-rw-r--r--cmd/podman/utils.go99
1 files changed, 42 insertions, 57 deletions
diff --git a/cmd/podman/utils.go b/cmd/podman/utils.go
index 522b74926..25bb36b60 100644
--- a/cmd/podman/utils.go
+++ b/cmd/podman/utils.go
@@ -1,6 +1,7 @@
package main
import (
+ "context"
"fmt"
"os"
gosignal "os/signal"
@@ -19,6 +20,7 @@ type RawTtyFormatter struct {
// Attach to a container
func attachCtr(ctr *libpod.Container, stdout, stderr, stdin *os.File, detachKeys string, sigProxy bool) error {
+ ctx := context.Background()
resize := make(chan remotecommand.TerminalSize)
haveTerminal := terminal.IsTerminal(int(os.Stdin.Fd()))
@@ -28,12 +30,10 @@ func attachCtr(ctr *libpod.Container, stdout, stderr, stdin *os.File, detachKeys
if haveTerminal && ctr.Spec().Process.Terminal {
logrus.Debugf("Handling terminal attach")
- resizeTerminate := make(chan interface{})
- defer func() {
- resizeTerminate <- true
- close(resizeTerminate)
- }()
- resizeTty(resize, resizeTerminate)
+ subCtx, cancel := context.WithCancel(ctx)
+ defer cancel()
+
+ resizeTty(subCtx, resize)
oldTermState, err := term.SaveState(os.Stdin.Fd())
if err != nil {
@@ -46,19 +46,6 @@ func attachCtr(ctr *libpod.Container, stdout, stderr, stdin *os.File, detachKeys
defer restoreTerminal(oldTermState)
}
- // There may be on the other size of the resize channel a goroutine trying to send.
- // So consume all data inside the channel before exiting to avoid a deadlock.
- defer func() {
- for {
- select {
- case <-resize:
- logrus.Debugf("Consumed resize command from channel")
- default:
- return
- }
- }
- }()
-
streams := new(libpod.AttachStreams)
streams.OutputStream = stdout
streams.ErrorStream = stderr
@@ -89,6 +76,7 @@ func attachCtr(ctr *libpod.Container, stdout, stderr, stdin *os.File, detachKeys
// Start and attach to a container
func startAttachCtr(ctr *libpod.Container, stdout, stderr, stdin *os.File, detachKeys string, sigProxy bool) error {
+ ctx := context.Background()
resize := make(chan remotecommand.TerminalSize)
haveTerminal := terminal.IsTerminal(int(os.Stdin.Fd()))
@@ -98,12 +86,10 @@ func startAttachCtr(ctr *libpod.Container, stdout, stderr, stdin *os.File, detac
if haveTerminal && ctr.Spec().Process.Terminal {
logrus.Debugf("Handling terminal attach")
- resizeTerminate := make(chan interface{})
- defer func() {
- resizeTerminate <- true
- close(resizeTerminate)
- }()
- resizeTty(resize, resizeTerminate)
+ subCtx, cancel := context.WithCancel(ctx)
+ defer cancel()
+
+ resizeTty(subCtx, resize)
oldTermState, err := term.SaveState(os.Stdin.Fd())
if err != nil {
@@ -116,19 +102,6 @@ func startAttachCtr(ctr *libpod.Container, stdout, stderr, stdin *os.File, detac
defer restoreTerminal(oldTermState)
}
- // There may be on the other size of the resize channel a goroutine trying to send.
- // So consume all data inside the channel before exiting to avoid a deadlock.
- defer func() {
- for {
- select {
- case <-resize:
- logrus.Debugf("Consumed resize command from channel")
- default:
- return
- }
- }
- }()
-
streams := new(libpod.AttachStreams)
streams.OutputStream = stdout
streams.ErrorStream = stderr
@@ -171,33 +144,45 @@ func startAttachCtr(ctr *libpod.Container, stdout, stderr, stdin *os.File, detac
return nil
}
+// getResize returns a TerminalSize command matching stdin's current
+// size on success, and nil on errors.
+func getResize() *remotecommand.TerminalSize {
+ winsize, err := term.GetWinsize(os.Stdin.Fd())
+ if err != nil {
+ logrus.Warnf("Could not get terminal size %v", err)
+ return nil
+ }
+ return &remotecommand.TerminalSize{
+ Width: winsize.Width,
+ Height: winsize.Height,
+ }
+}
+
// Helper for prepareAttach - set up a goroutine to generate terminal resize events
-func resizeTty(resize chan remotecommand.TerminalSize, resizeTerminate chan interface{}) {
+func resizeTty(ctx context.Context, resize chan remotecommand.TerminalSize) {
sigchan := make(chan os.Signal, 1)
gosignal.Notify(sigchan, signal.SIGWINCH)
- sendUpdate := func() {
- winsize, err := term.GetWinsize(os.Stdin.Fd())
- if err != nil {
- logrus.Warnf("Could not get terminal size %v", err)
- return
- }
- resize <- remotecommand.TerminalSize{
- Width: winsize.Width,
- Height: winsize.Height,
- }
- }
go func() {
defer close(resize)
// Update the terminal size immediately without waiting
// for a SIGWINCH to get the correct initial size.
- sendUpdate()
+ resizeEvent := getResize()
for {
- select {
- case <-resizeTerminate:
- return
-
- case <-sigchan:
- sendUpdate()
+ if resizeEvent == nil {
+ select {
+ case <-ctx.Done():
+ return
+ case <-sigchan:
+ resizeEvent = getResize()
+ }
+ } else {
+ select {
+ case <-ctx.Done():
+ return
+ case <-sigchan:
+ resizeEvent = getResize()
+ case resize <- *resizeEvent:
+ }
}
}
}()