Skip to content

Commit 79442ec

Browse files
committed
Fix -stdin being blocked after a restart until Enter is pressed.
Closes #22. os.Stdin.Read() blocks until a newline `\n` is provided (due to the OS doing line-by-line buffering https://groups.google.com/g/golang-nuts/c/NvD-pOTASIk). Therefore, do not tie the lifecycle of a command with os.Stdin.Read(). Instead, read from os.Stdin continuously in a background goroutine and feed its data into the stdinPipe of whichever exec.Cmd is currently running.
1 parent 7b2aeb7 commit 79442ec

File tree

2 files changed

+57
-19
lines changed

2 files changed

+57
-19
lines changed

wgo_cmd.go

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ type WgoCmd struct {
111111
// EnableStdin controls whether the Stdin field is used.
112112
EnableStdin bool
113113

114-
// Stdin is where the last command gets its stdin input from (EnableStdin
115-
// must be true).
114+
// Stdin is where commands get stdin input from (EnableStdin must be true).
116115
Stdin io.Reader
117116

118117
// Stdout is where the commands write their stdout output.
@@ -425,6 +424,31 @@ func (wgoCmd *WgoCmd) Run() error {
425424
}
426425
defer timer.Stop()
427426

427+
// Start a background job that continuously drains data from os.Stdin and
428+
// feeds it into stdinPipe (which connected to an exec.Cmd). stdinPipe can
429+
// be swapped out anytime when the exec.Cmd changes, so access is guarded
430+
// by a mutex.
431+
var stdinPipe io.WriteCloser
432+
var stdinPipeMutex sync.Mutex
433+
if wgoCmd.EnableStdin {
434+
go func() {
435+
p := make([]byte, 4096)
436+
for {
437+
n, err := wgoCmd.Stdin.Read(p)
438+
if n > 0 {
439+
stdinPipeMutex.Lock()
440+
if stdinPipe != nil {
441+
_, _ = stdinPipe.Write(p[:n])
442+
}
443+
stdinPipeMutex.Unlock()
444+
}
445+
if err != nil {
446+
break
447+
}
448+
}
449+
}()
450+
}
451+
428452
for restartCount := 0; ; restartCount++ {
429453
CMD_CHAIN:
430454
for i, args := range wgoCmd.ArgsList {
@@ -494,26 +518,19 @@ func (wgoCmd *WgoCmd) Run() error {
494518
}
495519
}
496520
// If the user enabled it, feed wgoCmd.Stdin to the command's
497-
// Stdin. Only the last command gets to read from Stdin -- if we
498-
// give Stdin to every command in the middle it will prevent the
499-
// next command from being executed if they don't consume Stdin.
521+
// Stdin.
500522
//
501523
// We have to use cmd.StdinPipe() here instead of assigning
502524
// cmd.Stdin directly, otherwise `wgo run ./testdata/stdin` doesn't
503525
// work interactively (the tests will pass, but somehow it won't
504526
// actually work if you run it in person. I don't know why).
505-
var wg sync.WaitGroup
506-
if wgoCmd.EnableStdin && i == len(wgoCmd.ArgsList)-1 {
507-
stdinPipe, err := cmd.StdinPipe()
527+
if wgoCmd.EnableStdin {
528+
stdinPipeMutex.Lock()
529+
stdinPipe, err = cmd.StdinPipe()
530+
stdinPipeMutex.Unlock()
508531
if err != nil {
509532
return err
510533
}
511-
wg.Add(1)
512-
go func() {
513-
defer wg.Done()
514-
defer stdinPipe.Close()
515-
_, _ = io.Copy(stdinPipe, wgoCmd.Stdin)
516-
}()
517534
}
518535

519536
// Step 2: Run the command in the background.
@@ -525,9 +542,8 @@ func (wgoCmd *WgoCmd) Run() error {
525542
return err
526543
}
527544
go func() {
528-
wg.Wait()
545+
defer close(waitDone)
529546
cmdResult <- cmd.Wait()
530-
close(waitDone)
531547
}()
532548

533549
// Step 3: Wait for events in the event loop.

wgo_cmd_test.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"context"
66
"errors"
77
"flag"
8+
"io"
89
"log"
910
"math/rand"
1011
"os"
@@ -862,16 +863,37 @@ func TestStdin(t *testing.T) {
862863
t.Parallel()
863864
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
864865
defer cancel()
865-
wgoCmd, err := WgoCommand(ctx, 0, []string{"run", "-exit", "-dir", "testdata/stdin", "-stdin", "./testdata/stdin"})
866+
// Precompile ./testdata/stdin executable so that wgoCmd.Run() isn't
867+
// affected by Go compile times. We need to command to run _immediately_ so
868+
// that it starts reading from stdin immediately.
869+
executablePath := filepath.Join(os.TempDir(), t.Name()+".exe")
870+
err := exec.Command("go", "build", "-o", executablePath, "./testdata/stdin").Run()
866871
if err != nil {
867872
t.Fatal(err)
868873
}
869-
wgoCmd.Stdin = strings.NewReader("foo\nbar\nbaz")
874+
defer os.Remove(executablePath)
875+
wgoCmd, err := WgoCommand(ctx, 0, []string{"-dir", "testdata/stdin", "-stdin", executablePath})
876+
if err != nil {
877+
t.Fatal(err)
878+
}
879+
// This is so awfully flaky, but we probably need to wait like 5 seconds
880+
// before writing to stdin in case the command hasn't started. Just in
881+
// case, idk, the test runner is running on a pentium cpu or something and
882+
// causes tests to fail because it's too slow.
883+
pipeReader, pipeWriter := io.Pipe()
884+
wgoCmd.Stdin = pipeReader
885+
go func() {
886+
time.Sleep(5 * time.Second)
887+
pipeWriter.Write([]byte("foo\nbar\nbaz\n"))
888+
pipeWriter.Close()
889+
}()
870890
buf := &Buffer{}
871891
wgoCmd.Stderr = buf
872892
err = wgoCmd.Run()
873893
if err != nil {
874-
t.Fatal(err)
894+
if !errors.Is(err, context.DeadlineExceeded) {
895+
t.Fatal(err)
896+
}
875897
}
876898
got := strings.TrimSpace(buf.String())
877899
want := "1: foo\n2: bar\n3: baz"

0 commit comments

Comments
 (0)