Skip to content

GH-909: Fix KexOutputHandler self-deadlock on inline write failure during shutdown#910

Merged
tomaswolf merged 1 commit into
apache:dev_3.0from
zimmermq:GH-909-kexoutputhandler-deadlock
Jul 23, 2026
Merged

GH-909: Fix KexOutputHandler self-deadlock on inline write failure during shutdown#910
tomaswolf merged 1 commit into
apache:dev_3.0from
zimmermq:GH-909-kexoutputhandler-deadlock

Conversation

@zimmermq

Copy link
Copy Markdown

Fixes #909.

When a write started under the read lock in writeOrEnqueue() fails inline, the session close re-enters KexOutputHandler.shutdown()updateState()`, which tries to take the write lock on the same thread — an illegal read-to-write upgrade that self-deadlocks.

Fix: in shutdown(), when the current thread already holds the read lock (getReadHoldCount() > 0), update the state without acquiring the write lock. The read lock already excludes writers, so it stays consistent; KEX write-ordering is unchanged.

Includes a deterministic regression test (hangs without the fix, passes with it).

@tomaswolf

Copy link
Copy Markdown
Member

Can you please put the PR description minus the "Fixes #909" line into the commit message?

Good commit messages are important, especially in OSS. For future contributors and maintainers it is very helpful if the commit messages explain the reasoning behind a change as it makes it much easier to figure out the history via git blame. If only the PR explains it, people would have to navigate away from git to find the PR first to learn why exactly a certain change was done. (OK, in this case there's also the inline comment, but still.)

@zimmermq
zimmermq force-pushed the GH-909-kexoutputhandler-deadlock branch from 599727f to 36f6faf Compare July 23, 2026 08:58
@zimmermq

Copy link
Copy Markdown
Author

Done, I've moved the explanation into the commit message (subject + body) and force-pushed. Thanks for the nudge; fully agree the reasoning belongs in git rather than only in the PR.

…ure during shutdown

When a write started under the read lock in writeOrEnqueue() fails inline, the session close re-enters KexOutputHandler.shutdown()→updateState(), which tries to take the write lock on the same thread — an illegal read-to-write upgrade that self-deadlocks.

Fix: in shutdown(), when the current thread already holds the read lock (getReadHoldCount() > 0), update the state without acquiring the write lock. The read lock already excludes writers, so it stays consistent; KEX write-ordering is unchanged.

Includes a deterministic regression test (hangs without the fix, passes with it).
@zimmermq
zimmermq force-pushed the GH-909-kexoutputhandler-deadlock branch from 36f6faf to da979e1 Compare July 23, 2026 09:17
@tomaswolf
tomaswolf merged commit d5f1b73 into apache:dev_3.0 Jul 23, 2026
7 checks passed
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.

2 participants