10000 fix: cap max X11 forwarding ports and evict old by spikecurtis · Pull Request #18561 · coder/coder · GitHub
[go: up one dir, main page]

Skip to content

fix: cap max X11 forwarding ports and evict old #18561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 27, 2025

Conversation

spikecurtis
Copy link
Contributor
@spikecurtis spikecurtis commented Jun 25, 2025

partial for #18263

Caps the X11 forwarding sessions at a maximum port of 6200, and evicts the oldest session if we create new sessions while at the max.

Unit tests included higher in the stack.

Copy link
Member
@mafredri mafredri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good refactor, nice work 👍🏻. I'm however not entirely sold on the lru eviction policy since it's based on connection establishment time and not connection usage (i.e. we may end evicting a busy connection). (See related comment for additional context.)

@spikecurtis spikecurtis force-pushed the spike/18263-cap-x11-forwarding-ports branch from 77f7937 to 367d38c Compare June 26, 2025 10:41
@spikecurtis spikecurtis requested a review from mafredri June 26, 2025 10:42
Copy link
Member
@mafredri mafredri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raised two points inline and we should at least fix the nil check, but once that's fixed I think this is good to go, approved. 👍🏻

@spikecurtis spikecurtis force-pushed the spike/18263-cap-x11-forwarding-ports branch from 367d38c to 4c219b3 Compare June 27, 2025 09:42
@spikecurtis spikecurtis merged commit 9e1cf16 into main Jun 27, 2025
54 of 60 checks passed
Copy link
Contributor Author

Merge activity

@spikecurtis spikecurtis deleted the spike/18263-cap-x11-forwarding-ports branch June 27, 2025 10:05
@github-actions github-actions bot locked and limited conversation to collaborators Jun 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0