Skip to content

Fix channel-open cancellation deadlock in ConnectionService#154

Open
juanpacostaaa wants to merge 3 commits into
microsoft:mainfrom
juanpacostaaa:dev/juanpacostaaa/fix-channel-cancellation-deadlock
Open

Fix channel-open cancellation deadlock in ConnectionService#154
juanpacostaaa wants to merge 3 commits into
microsoft:mainfrom
juanpacostaaa:dev/juanpacostaaa/fix-channel-cancellation-deadlock

Conversation

@juanpacostaaa

Copy link
Copy Markdown

Problem

ConnectionService.HandleMessageAsync disposed a pending channel's CancellationTokenRegistration
while holding lockObject. The callback registered in OpenChannelAsync also takes lockObject,
and Dispose() blocks until an in-flight callback completes. So an open cancelling exactly as its
confirmation was processed deadlocked the pump thread, hanging every later OpenChannelAsync

Fix

Dispose the registration after releasing the lock. Removing the pending channel under the lock
makes the callback a no-op, so deferring the dispose is safe. Applied to the confirmation and
failure handlers.

Tests

Added OpenChannelCancelDuringConfirmationDoesNotDeadlock and SessionRemainsUsableAfterOpenCancelRace,
which force the race via a small internal hook reached by reflection (like ReconnectTests does for
KeyRotationThreshold).

…ion after releasing the lock.

ConnectionService.HandleMessageAsync was disposing a pending channel's CancellationTokenRegistration while holding lockObject, but the callback registered in OpenChannelAsync also took lockObject, and Dispose() blocks until an in-flight callback
completes — so a channel-open cancelling at that moment deadlocked the session and leaked
every later OpenChannelAsync thread.
…ancellation deadlock, using a small internal ConnectionService hook reached by reflection (matching how ReconnectTests reaches KeyRotationThreshold) to force the cancel-during-confirmation race.
adamasmar
adamasmar previously approved these changes Jul 23, 2026
Comment thread test/cs/Ssh.Test/ChannelTests.cs Outdated
@juanpacostaaa
juanpacostaaa force-pushed the dev/juanpacostaaa/fix-channel-cancellation-deadlock branch from a97004e to 00e876b Compare July 23, 2026 22:30
…ook with an explicit synchronization handshake using TaskCompletionSource and CancellationToken LIFO callback ordering. A second callback registered after the internal one fires first (LIFO), and once it completes, Cancel() immediately proceeds to the internal callback on the same thread — which blocks on lockObject.
@juanpacostaaa
juanpacostaaa requested a review from klvnraju July 23, 2026 22:54
@juanpacostaaa

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Microsoft"

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants