fix/conn: avoid deadlock in re-entrant protocol error logging - #99
Merged
Conversation
Protocol errors invoke the configured logger while the connection is shutting down. Calling user code under the connection mutex meant a logger that sends over the same connection could never observe closure and instead deadlocked. Complete teardown before logging so re-entrant calls return ErrClosed. Amp-Thread-ID: https://ampcode.com/threads/T-019fb774-aef7-75df-8eb3-8888d6835754 Co-authored-by: Amp <amp@ampcode.com>
burmudar
approved these changes
Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A custom logger can call back into the same JSON-RPC connection—for example, to forward logs to the peer. When malformed input caused a protocol error, connection shutdown previously invoked that logger while holding the connection mutex. If the logger called `Notify`, it attempted to reacquire the mutex and deadlocked before shutdown could complete.
This marks the connection closed and releases its mutex before invoking the logger. Shutdown and transport cleanup complete first, so a re-entrant call now returns `ErrClosed` rather than blocking. A regression test reproduces the reported setup and verifies that both the logger and disconnect notification complete.
Fixes #98.