Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix quit on ctrlc, race panic, atomic load align in session IO #11112

Merged
merged 10 commits into from
Mar 15, 2022

Conversation

xacrimon
Copy link
Contributor

lib/srv/termmanager.go is responsible for dealing with most session IO and handles behaviours like multiplexing. This PR fixes the following bugs:

This PR does remove some usage of atomics in termmanager and instead locks additional fields under the mutex. I didn't observe any noticable performance drop here and this would only become a problem if you had hundreds of users typing in a session at the same time. This makes the code somewhat easier to reason about but we'll have to look into that again if it becomes a problem at some point in the future.

@rosstimothy
Copy link
Contributor

It doesn't look like lib/srv/termmanager.go has much test coverage at all. Could we at least add some for the issues this is fixing to prevent any regressions here?

@xacrimon
Copy link
Contributor Author

xacrimon commented Mar 14, 2022

@rosstimothy I wasn't able to trigger the race condition that was potentially possible due to tight tolerances or some other reason like optimization but I've added tests for CTRLC capture and atomic alignment. Please have another look

@xacrimon xacrimon requested a review from jakule March 15, 2022 08:51
@xacrimon xacrimon enabled auto-merge (squash) March 15, 2022 12:09
@webvictim webvictim mentioned this pull request Jun 8, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Teleport panics with unaligned 64-bit atomic operation on 32bit ARM CTRL-C causing sessions to freeze on 9.0
4 participants