Skip to content

Commit

Permalink
fix(overlord): send EOF to master pty in interactive mode (#466)
Browse files Browse the repository at this point in the history
If the interactive mode is enforced in `pebble exec`, there are
scenarios like:

```go run ./cmd/pebble exec -i -- tee < ./HACKING.md```

that would prompt the server to shutdown the execution earlier than the whole output from 'tee' will be read and sent to the client. Hence, instead of closing the master pty descriptor when there is no more input from the client, send
Ctrl-D (EOF) to indicate to the process that there will be no more input.

Fixes #306.
  • Loading branch information
dmitry-lyfar authored Aug 6, 2024
1 parent d3a4408 commit dcd37f3
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
7 changes: 6 additions & 1 deletion internals/overlord/cmdstate/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,12 @@ func (e *execution) do(ctx context.Context, task *state.Task) error {
// websocket and write to the PTY.
go func() {
<-wsutil.WebsocketRecvStream(master, ioConn)
master.Close()
// If the interactive is enforced, it is possible to finish
// reading earlier than the mirroring go routine sends all the
// output to the client. Thus, closing the master descriptor
// here will terminate mirroring prematurely. Instead, we
// should send Ctrl-D to the fd to indicate the end of input.
master.Write([]byte{byte(unix.VEOF)})
}()
} else {
// Non-interactive: start goroutine to receive stdin from "stdio"
Expand Down
2 changes: 1 addition & 1 deletion internals/wsutil/wsutil_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func MirrorToWebsocket(conn MessageWriter, r io.ReadCloser, exited chan struct{}
for {
buf, ok := <-in
if !ok {
r.Close()
_ = r.Close()
logger.Debugf("Sending write barrier")
err := conn.WriteMessage(websocket.TextMessage, endCommandJSON)
if err != nil {
Expand Down

0 comments on commit dcd37f3

Please sign in to comment.