Skip to content

Prevent deadlock on moderated sessions when mod connection drops#36882

Merged
tigrato merged 1 commit intomasterfrom
tigrato/fix-moderated-sessions-deadlock
Jan 19, 2024
Merged

Prevent deadlock on moderated sessions when mod connection drops#36882
tigrato merged 1 commit intomasterfrom
tigrato/fix-moderated-sessions-deadlock

Conversation

@tigrato
Copy link
Copy Markdown
Contributor

@tigrato tigrato commented Jan 18, 2024

This PR removes a deadlock caused by the moderator leaving the session because his connection drops.

OnWriteError was called under lock which creates an issue if the function calls Termnager.DeleteWriter to exclude the writer from the term manager.

This PR also correctly forwards errors to the clients when they occur.

Changelog: Ensure that moderated sessions do not get stuck in the event of an unexpected drop in the moderator's connection.

Fixes #36881

Comment thread lib/kube/proxy/forwarder.go Outdated
Comment thread lib/kube/proxy/forwarder.go Outdated
Comment thread lib/kube/proxy/forwarder.go Outdated
Comment thread lib/kube/proxy/forwarder.go Outdated
Comment thread lib/kube/proxy/forwarder.go Outdated
Comment thread lib/srv/termmanager.go Outdated
Comment thread lib/srv/termmanager.go Outdated
Comment thread lib/srv/termmanager.go Outdated
Comment thread lib/srv/termmanager.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any change we could get a test for the deadlock?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would like to have but it's not that easy to have reproducible tests with this

@tigrato tigrato requested a review from codingllama January 18, 2024 20:00
Comment thread lib/kube/proxy/forwarder.go Outdated
Comment thread lib/kube/proxy/forwarder.go Outdated
This PR removes a deadlock caused by the moderator leaving the session
because his connection drops.

`OnWriteError` was called under lock which creates an issue if the
function calls `Termnager.DeleteWriter` to exclude the writter from the
term manager.

This PR also correctly forwards errors to the clients when they occur.

Changelog: Ensure that moderated sessions do not get stuck in the event of an unexpected drop in the moderator's connection.

Fixes #36881

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
@tigrato tigrato force-pushed the tigrato/fix-moderated-sessions-deadlock branch from e778b86 to 5bb61d3 Compare January 19, 2024 10:00
@tigrato tigrato added this pull request to the merge queue Jan 19, 2024
Merged via the queue into master with commit f57dc2f Jan 19, 2024
@tigrato tigrato deleted the tigrato/fix-moderated-sessions-deadlock branch January 19, 2024 10:55
@public-teleport-github-review-bot
Copy link
Copy Markdown

@tigrato See the table below for backport results.

Branch Result
branch/v12 Create PR
branch/v13 Create PR
branch/v14 Create PR
branch/v15 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Moderated sessions end up in deadlock when moderator leaves by network problems

3 participants