8000 chore: update X11 forward session usage when there is a connection (#… · coder/coder@a02d5a6 · GitHub
[go: up one dir, main page]

Skip to content

Commit a02d5a6

Browse files
authored
chore: update X11 forward session usage when there is a connection (#18567)
fixes #18263 Adds support to bump `usedAt` for X11 forwarding sessions whenever an application connects over the TCP socket. This should help avoid evicting sessions that are actually in use.
1 parent 73c742a commit a02d5a6

File tree

2 files changed

+53
-8
lines changed

2 files changed

+53
-8
lines changed

agent/agentssh/x11.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,11 @@ func (x *x11Forwarder) listenForConnections(
162162
x.logger.Warn(ctx, "failed to accept X11 connection", slog.Error(err))
163163
return
164164
}
165+
166+
// Update session usage time since a new X11 connection was forwarded.
167+
x.mu.Lock()
168+
session.usedAt = time.Now()
169+
x.mu.Unlock()
165170
if x11.SingleConnection {
166171
x.logger.Debug(ctx, "single connection requested, closing X11 listener")
167172
x.closeAndRemoveSession(session)

agent/agentssh/x11_test.go

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,31 @@ func TestServer_X11_EvictionLRU(t *testing.T) {
232232
})
233233
}
234234

235-
// Create one more session which should evict the first (LRU) session and
236-
// therefore reuse the very first display/port.
235+
// Connect X11 forwarding to the first session. This is used to test that
236+
// connecting counts as a use of the display.
237+
x11Chans := c.HandleChannelOpen("x11")
238+
payload := "hello world"
239+
go func() {
240+
conn, err := inproc.Dial(ctx, testutil.NewAddr("tcp", fmt.Sprintf("localhost:%d", agentssh.X11StartPort+agentssh.X11DefaultDisplayOffset)))
241+
assert.NoError(t, err)
242+
_, err = conn.Write([]byte(payload))
243+
assert.NoError(t, err)
244+
_ = conn.Close()
245+
}()
246+
247+
x11 := testutil.RequireReceive(ctx, t, x11Chans)
248+
ch, reqs, err := x11.Accept()
249+
require.NoError(t, err)
250+
go gossh.DiscardRequests(reqs)
251+
got := make([]byte, len(payload))
252+
_, err = ch.Read(got)
253+
require.NoError(t, err)
254+
assert.Equal(t, payload, string(got))
255+
_ = ch.Close()
256+
257+
// Create one more session which should evict a session and reuse the display.
258+
// The first session was used to connect X11 forwarding, so it should not be evicted.
259+
// Therefore, the second session should be evicted and its display reused.
237260
extraSess, err := c.NewSession()
238261
require.NoError(t, err)
239262

@@ -271,17 +294,34 @@ func TestServer_X11_EvictionLRU(t *testing.T) {
271294
require.NoError(t, sc.Err())
272295
}
273296

274-
// The display number should have wrapped around to the starting value.
275-
assert.Equal(t, agentssh.X11DefaultDisplayOffset, newDisplayNumber, "expected display number to be reused after eviction")
297+
// The display number reused should correspond to the SECOND session (display offset 12)
298+
expectedDisplay := agentssh.X11DefaultDisplayOffset + 2 // +1 was blocked port
299+
assert.Equal(t, expectedDisplay, newDisplayNumber, "second session should have been evicted and its display reused")
276300

277-
// validate that the first session was torn down.
278-
_, err = sessions[0].stdin.Write([]byte("echo DISPLAY=$DISPLAY\n"))
301+
// First session should still be alive: send cmd and read output.
302+
msgFirst := "still-alive"
303+
_, err = sessions[0].stdin.Write([]byte("echo " + msgFirst + "\n"))
304+
require.NoError(t, err)
305+
for sessions[0].scanner.Scan() {
306+
line := strings.TrimSpace(sessions[0].scanner.Text())
307+
if strings.Contains(line, msgFirst) {
308+
break
309+
}
310+
}
311+
require.NoError(t, sessions[0].scanner.Err())
312+
313+
// Second session should now be closed.
314+
_, err = sessions[1].stdin.Write([]byte("echo dead\n"))
279315
require.ErrorIs(t, err, io.EOF)
280-
err = sessions[0].sess.Wait()
316+
err = sessions[1].sess.Wait()
281317
require.Error(t, err)
282318

283319
// Cleanup.
284-
for _, sh := range sessions[1:] {
320+
for i, sh := range sessions {
321+
if i == 1 {
322+
// already closed
323+
continue
324+
}
285325
err = sh.stdin.Close()
286326
require.NoError(t, err)
287327
err = sh.sess.Wait()

0 commit comments

Comments
 (0)
0