Skip to content

Commit

Permalink
Make Wash daemon part of the shell session
Browse files Browse the repository at this point in the history
When `wash` is run, it starts a shell as a child process where all the
action happens. It then stays running as a daemon to handle
Wash-specific requests through FUSE and the socket API. However if it
needs to prompt for input, it has trouble because it's TTY is not the
same as the one handling input (the child shell).

We previously addressed that by temporarily transferring control of
input to the Wash daemon, but that had some problems
- it only worked when a plugin requested input using Wash's helper
- if the process that triggered the plugin activity tried to do
something with stdin - like MacVim appears to on startup - it would fail

Reparent the Wash daemon to have a new session under the new shell so
they share the same TTY. Prompts for input now work wherever they're
triggered from. MacVim still behaves a little strangely if you get a
prompt when opening it, but I'm not sure what to do about it.

Fixes puppetlabs-toy-chest#623.

Signed-off-by: Michael Smith <[email protected]>
  • Loading branch information
MikaelSmith committed Dec 3, 2019
1 parent fc97023 commit 836c360
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 56 deletions.
88 changes: 73 additions & 15 deletions cmd/rootMain.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import (
"io/ioutil"
"os"
"os/exec"
"os/signal"
"path/filepath"
"strings"
"syscall"
"time"

"golang.org/x/sys/unix"

"github.com/puppetlabs/wash/cmd/internal/server"
"github.com/puppetlabs/wash/cmd/internal/shell"
Expand All @@ -26,13 +27,6 @@ func rootMain(cmd *cobra.Command, args []string) exitCode {
// Configure logrus to emit simple text
log.SetFormatter(&log.TextFormatter{DisableTimestamp: true})

// When Wash prompts for input but it doesn't currently control STDIN, it will get SIGTTOU. This
// is common when a plugin prompts for input in response to running a command like `ls` or `cat`.
// Ignore this signal so we aren't suspended when prompting for input from the background.
// We also ignore SIGTTIN because http://curiousthing.org/sigttin-sigttou-deep-dive-linux
// suggests that's the signal we should be getting, even though I haven't seen it in testing.
signal.Ignore(syscall.SIGTTOU, syscall.SIGTTIN)

cachedir, ok := makeCacheDir()
if !ok {
return exitCode{1}
Expand Down Expand Up @@ -78,6 +72,7 @@ func rootMain(cmd *cobra.Command, args []string) exitCode {
cmdutil.ErrPrintf("Unable to start server: %v\n", err)
return exitCode{1}
}
defer srv.Stop()

if plugin.IsInteractive() {
cmdutil.Println(`Welcome to Wash!
Expand Down Expand Up @@ -122,16 +117,79 @@ Try 'help'`)
)
comm.Dir = mountpath

// Inspired by https://blog.nelhage.com/2011/02/changing-ctty/. After the child shell has
// started, we re-parent the Wash daemon to be in a child session of the shell so that when Wash
// prompts for input it's within the TTY of that terminal.
if startErr := comm.Start(); startErr != nil {
cmdutil.ErrPrintf("%v\n", startErr)
return exitCode{1}
}

codeCh := make(chan int)
go func() {
if runErr := comm.Wait(); runErr != nil {
if exitErr, ok := runErr.(*exec.ExitError); ok {
codeCh <- exitErr.ExitCode()
} else {
cmdutil.SafeErrPrintf("%v\n", runErr)
codeCh <- 1
}
} else {
codeCh <- 0
}
close(codeCh)
}()

var exit exitCode
if runErr := comm.Run(); runErr != nil {
if exitErr, ok := runErr.(*exec.ExitError); ok {
exit.value = exitErr.ExitCode()
killShellProcess := func() exitCode {
if err := comm.Process.Kill(); err != nil {
cmdutil.SafeErrPrintf("Couldn't stop child shell: %v\n", err)
}
cmdutil.ErrPrintf("%v\n", runErr)
exit.value = 1
return exitCode{1}
}

srv.Stop()
// The new shell will initially be part of our process group. We wait for it to become the leader
// of its own process group, then move Wash to be a new session under that process group.
washPid := os.Getpid()
pid := comm.Process.Pid
for {
time.Sleep(10 * time.Millisecond)

select {
case code := <-codeCh:
// Child shell stopped, so just exit with it's exit code.
exit.value = code
break
default:
// Fall-through
}

pgid, err := unix.Getpgid(pid)
if err != nil {
cmdutil.SafeErrPrintf("Error moving Wash daemon to new shell: %v\n", err)
return killShellProcess()
}

// Once the shell is the leader of its own process group, move Wash to that group and
// put it in a new session.
if pgid == pid {
if err := unix.Setpgid(washPid, pgid); err != nil {
cmdutil.SafeErrPrintf("Error moving Wash daemon to new shell: %v\n", err)
return killShellProcess()
}

if _, err := unix.Setsid(); err != nil {
cmdutil.SafeErrPrintf("Error starting new session for Wash daemon: %v\n", err)
return killShellProcess()
}

break
}
}

if code, ok := <-codeCh; ok {
exit.value = code
}
if plugin.IsInteractive() {
cmdutil.Println("Goodbye!")
}
Expand Down
43 changes: 2 additions & 41 deletions plugin/interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ import (
"fmt"
"os"
"sync"
"unsafe"

"github.com/mattn/go-isatty"
"golang.org/x/sys/unix"
)

var isInteractive bool = (isatty.IsTerminal(os.Stdin.Fd()) || isatty.IsCygwinTerminal(os.Stdin.Fd())) &&
Expand All @@ -25,16 +23,6 @@ func IsInteractive() bool {
return isInteractive
}

func tcGetpgrp(fd int) (pgrp int, err error) {
return unix.IoctlGetInt(fd, unix.TIOCGPGRP)
}

func tcSetpgrp(fd int, pgrp int) (err error) {
// Mimic IoctlSetPointerInt, which is not available on macOS.
v := int32(pgrp)
return unix.IoctlSetInt(fd, unix.TIOCSPGRP, int(uintptr(unsafe.Pointer(&v))))
}

// Only allow one Prompt call at a time. This prevents multiple plugins loading concurrently
// from messing things up by calling Prompt concurrently.
var promptMux sync.Mutex
Expand All @@ -48,35 +36,8 @@ func Prompt(msg string) (string, error) {
promptMux.Lock()
defer promptMux.Unlock()

// Even if Wash is running interactively, it will not have control of STDIN while another command
// is running within the shell environment. If it doesn't have control and tries to read from it,
// the read will fail. If we have control, read normally. If not, temporarily acquire control for
// the current process group while we're prompting, then return it afterward so the triggering
// command can continue.
inFd := int(os.Stdin.Fd())
inGrp, err := tcGetpgrp(inFd)
if err != nil {
return "", fmt.Errorf("error getting process group controlling stdin: %v", err)
}
curGrp := unix.Getpgrp()

var v string
if inGrp == curGrp {
// We control stdin
fmt.Fprintf(os.Stderr, "%s: ", msg)
_, err = fmt.Scanln(&v)
} else {
// Need to get control, prompt, then return control.
if err := tcSetpgrp(inFd, curGrp); err != nil {
return "", fmt.Errorf("error getting control of stdin: %v", err)
}
fmt.Fprintf(os.Stderr, "%s: ", msg)
_, err = fmt.Scanln(&v)
if err := tcSetpgrp(inFd, inGrp); err != nil {
// Panic if we can't return control. A messed up environment that they 'kill -9' is worse.
panic(err.Error())
}
}
// Return the error set by Scanln.
fmt.Fprintf(os.Stderr, "%s: ", msg)
_, err := fmt.Scanln(&v)
return v, err
}

0 comments on commit 836c360

Please sign in to comment.