Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitb90c051

Browse files
committed
Immediately read screen process
It is possible for the process to immediately exit (for example if yourun `echo hello`), then we would wait for the `version` command tosucceed but it never will because the session is already gone.If we immediately read to the process then we can tell when it has goneaway and we can abort.
1 parent285b4e7 commitb90c051

File tree

3 files changed

+82
-52
lines changed

3 files changed

+82
-52
lines changed

‎agent/agent_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1720,6 +1720,18 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
17201720
line:=scanner3.Text()
17211721
t.Logf("bash tty stdout = %s",re.ReplaceAllString(line,""))
17221722
}
1723+
1724+
// Try a non-shell command. It should output then immediately exit.
1725+
netConn4,err:=conn.ReconnectingPTY(ctx,uuid.New(),100,100,"echo test")
1726+
require.NoError(t,err)
1727+
defernetConn4.Close()
1728+
1729+
scanner4:=bufio.NewScanner(netConn4)
1730+
require.True(t,hasLine(scanner4,matchEchoOutput),"find exit command")
1731+
forscanner4.Scan() {
1732+
line:=scanner4.Text()
1733+
t.Logf("bash tty stdout = %s",re.ReplaceAllString(line,""))
1734+
}
17231735
})
17241736
}
17251737
}

‎agent/reconnectingpty/buffered.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ func newBuffered(ctx context.Context, cmd *pty.Cmd, options *Options, logger slo
9494
}
9595
// Could have been killed externally or failed to start at all (command
9696
// not found for example).
97+
// TODO: Should we check the process's exit code in case the command was
98+
// invalid?
9799
rpty.Close("unable to read pty output, command might have exited")
98100
break
99101
}

‎agent/reconnectingpty/screen.go

Lines changed: 68 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,15 @@ func (rpty *screenReconnectingPTY) Attach(ctx context.Context, _ string, conn ne
162162

163163
goheartbeat(ctx,rpty.timer,rpty.timeout)
164164

165-
ptty,process,err:=rpty.doAttach(ctx,height,width,logger)
165+
ptty,process,err:=rpty.doAttach(ctx,conn,height,width,logger)
166166
iferr!=nil {
167+
iferrors.Is(err,context.Canceled) {
168+
// Likely the process was too short-lived and canceled the version command.
169+
// TODO: Is it worth distinguishing between that and a cancel from the
170+
// Attach() caller? Additionally, since this could also happen if
171+
// the command was invalid, should we check the process's exit code?
172+
returnnil
173+
}
167174
returnerr
168175
}
169176

@@ -180,53 +187,6 @@ func (rpty *screenReconnectingPTY) Attach(ctx context.Context, _ string, conn ne
180187
}
181188
}()
182189

183-
// Pipe pty -> conn.
184-
// We do not need to separately monitor for the process exiting. When it
185-
// exits, our ptty.OutputReader() will return EOF after reading all process
186-
// output.
187-
gofunc() {
188-
// Close the connection when the process exits. Log only for debugging
189-
// since the connection might have already closed on its own.
190-
deferfunc() {
191-
err:=conn.Close()
192-
iferr!=nil {
193-
logger.Debug(ctx,"closed connection with error",slog.Error(err))
194-
}
195-
}()
196-
buffer:=make([]byte,1024)
197-
for {
198-
read,err:=ptty.OutputReader().Read(buffer)
199-
iferr!=nil {
200-
// When the PTY is closed, this is triggered.
201-
// Error is typically a benign EOF, so only log for debugging.
202-
iferrors.Is(err,io.EOF) {
203-
logger.Debug(ctx,"unable to read pty output; screen might have exited",slog.Error(err))
204-
}else {
205-
logger.Warn(ctx,"unable to read pty output; screen might have exited",slog.Error(err))
206-
rpty.metrics.WithLabelValues("screen_output_reader").Add(1)
207-
}
208-
// The process might have died because the session itself died or it
209-
// might have been separately killed and the session is still up (for
210-
// example `exit` or we killed it when the connection closed). If the
211-
// session is still up we might leave the reconnecting pty in memory
212-
// around longer than it needs to be but it will eventually clean up
213-
// with the timer or context, or the next attach will respawn the screen
214-
// daemon which is fine too.
215-
break
216-
}
217-
part:=buffer[:read]
218-
_,err=conn.Write(part)
219-
iferr!=nil {
220-
// Connection might have been closed.
221-
iferrors.Unwrap(err).Error()!="endpoint is closed for send" {
222-
logger.Warn(ctx,"error writing to active conn",slog.Error(err))
223-
rpty.metrics.WithLabelValues("screen_write").Add(1)
224-
}
225-
break
226-
}
227-
}
228-
}()
229-
230190
// Pipe conn -> pty and block.
231191
readConnLoop(ctx,conn,ptty,rpty.metrics,logger)
232192
returnnil
@@ -235,7 +195,7 @@ func (rpty *screenReconnectingPTY) Attach(ctx context.Context, _ string, conn ne
235195
// doAttach spawns the screen client and starts the heartbeat. It exists
236196
// separately only so we can defer the mutex unlock which is not possible in
237197
// Attach since it blocks.
238-
func (rpty*screenReconnectingPTY)doAttach(ctx context.Context,height,widthuint16,logger slog.Logger) (pty.PTYCmd, pty.Process,error) {
198+
func (rpty*screenReconnectingPTY)doAttach(ctx context.Context,conn net.Conn,height,widthuint16,logger slog.Logger) (pty.PTYCmd, pty.Process,error) {
239199
// Ensure another attach does not come in and spawn a duplicate session.
240200
rpty.mutex.Lock()
241201
deferrpty.mutex.Unlock()
@@ -273,11 +233,63 @@ func (rpty *screenReconnectingPTY) doAttach(ctx context.Context, height, width u
273233
returnnil,nil,err
274234
}
275235

236+
// This context lets us abort the version command if the process dies.
237+
versionCtx,versionCancel:=context.WithCancel(ctx)
238+
deferversionCancel()
239+
240+
// Pipe pty -> conn and close the connection when the process exits.
241+
// We do not need to separately monitor for the process exiting. When it
242+
// exits, our ptty.OutputReader() will return EOF after reading all process
243+
// output.
244+
gofunc() {
245+
deferversionCancel()
246+
deferfunc() {
247+
err:=conn.Close()
248+
iferr!=nil {
249+
// Log only for debugging since the connection might have already closed
250+
// on its own.
251+
logger.Debug(ctx,"closed connection with error",slog.Error(err))
252+
}
253+
}()
254+
buffer:=make([]byte,1024)
255+
for {
256+
read,err:=ptty.OutputReader().Read(buffer)
257+
iferr!=nil {
258+
// When the PTY is closed, this is triggered.
259+
// Error is typically a benign EOF, so only log for debugging.
260+
iferrors.Is(err,io.EOF) {
261+
logger.Debug(ctx,"unable to read pty output; screen might have exited",slog.Error(err))
262+
}else {
263+
logger.Warn(ctx,"unable to read pty output; screen might have exited",slog.Error(err))
264+
rpty.metrics.WithLabelValues("screen_output_reader").Add(1)
265+
}
266+
// The process might have died because the session itself died or it
267+
// might have been separately killed and the session is still up (for
268+
// example `exit` or we killed it when the connection closed). If the
269+
// session is still up we might leave the reconnecting pty in memory
270+
// around longer than it needs to be but it will eventually clean up
271+
// with the timer or context, or the next attach will respawn the screen
272+
// daemon which is fine too.
273+
break
274+
}
275+
part:=buffer[:read]
276+
_,err=conn.Write(part)
277+
iferr!=nil {
278+
// Connection might have been closed.
279+
iferrors.Unwrap(err).Error()!="endpoint is closed for send" {
280+
logger.Warn(ctx,"error writing to active conn",slog.Error(err))
281+
rpty.metrics.WithLabelValues("screen_write").Add(1)
282+
}
283+
break
284+
}
285+
}
286+
}()
287+
276288
// Version seems to be the only command without a side effect (other than
277289
// making the version pop up briefly) so use it to wait for the session to
278290
// come up. If we do not wait we could end up spawning multiple sessions with
279291
// the same name.
280-
err=rpty.sendCommand(ctx,"version",nil)
292+
err=rpty.sendCommand(versionCtx,"version",nil)
281293
iferr!=nil {
282294
closeErr:=ptty.Close()
283295
ifcloseErr!=nil {
@@ -298,8 +310,9 @@ func (rpty *screenReconnectingPTY) doAttach(ctx context.Context, height, width u
298310
// command fails with an error matching anything in successErrors it will be
299311
// considered a success state (for example "no session" when quitting and the
300312
// session is already dead). The command will be retried until successful, the
301-
// timeout is reached, or the context ends in which case the context error is
302-
// returned together with the last error from the command.
313+
// timeout is reached, or the context ends. A canceled context will return the
314+
// canceled context's error as-is while a timed-out context returns together
315+
// with the last error from the command.
303316
func (rpty*screenReconnectingPTY)sendCommand(ctx context.Context,commandstring,successErrors []string)error {
304317
ctx,cancel:=context.WithTimeout(ctx,attachTimeout)
305318
defercancel()
@@ -352,6 +365,9 @@ func (rpty *screenReconnectingPTY) sendCommand(ctx context.Context, command stri
352365
for {
353366
select {
354367
case<-ctx.Done():
368+
iferrors.Is(ctx.Err(),context.Canceled) {
369+
returnctx.Err()
370+
}
355371
returnerrors.Join(ctx.Err(),lastErr)
356372
case<-ticker.C:
357373
ifdone:=run();done {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp