fix: evict stale pooled connection before each control-channel dial#60
Merged
Merged
Conversation
ILibWebClient pipelines the WS upgrade onto a kept-alive connection from its DataTable when one exists for the target address. If the gateway abortively closed that socket (or it died during a network blip), the queued upgrade fails instantly with no HTTP response (tls=down, elapsedMs=0) without ever touching the network, and every 4-6 min retry rides the same dead object indefinitely - the agent stays offline until process restart while other clients on the same host reconnect fine. Deleting the address entry before pipelining forces a fresh TCP+TLS connect per dial. Safe because the single-flight guard guarantees no in-flight control-channel request at this point and httpClientManager has no other users, so the call can only drop an idle stale socket. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIn ChangesStale Request Eviction
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
denys-gif
approved these changes
Jul 2, 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
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.
Problem
During the mikart mesh-offline incident (Jul 1-2), agents entered a permanent retry loop logging
Connection FAILED: No HTTP response (… tls=down, elapsedMs=0 …)every 4-6 minutes for 12+ hours, while/ws/natsconnections from the same machines to the same IP/LB kept working. Evidence that the failure is local to the meshagent process:elapsedMs=0: hundreds of consecutive attempts failed in the same millisecond as the dial — impossible for any network exchange.attempt=…-1); the one machine that never restarted (attempt counter 1→2533) never recovered.Mechanism
ILibWebClient_PipelineRequestEx2reuses a kept-alive connection from the manager'sDataTablewhen an entry exists for the target address (microstack/ILibWebClient.c"Previous connection exists!" path). If that pooled socket was abortively closed by the gateway (SO_LINGER=0 RST) or died during a network blip, the queued upgrade fails immediately with no HTTP bytes —MeshServer_ControlChannel_ConnectSinknever fires (hencetls=down), and each retry re-queues onto the same dead object.Fix
Call
ILibWebClient_DeleteRequests(agent->httpClientManager, &meshServer)immediately before pipelining the upgrade inMeshServer_ConnectEx, evicting any pooled connection for the target address so every dial performs a fresh TCP+TLS connect. Connection reuse buys nothing here: the control channel is one long-lived WebSocket, dialed at most once per hour when healthy.Safety
serverConnectionState == 0 && controlChannel == NULL && controlChannelRequest == NULLat this point, so there are no pending requests to abort — the call can only disconnect an idle stale socket.agent->httpClientManagerhas exactly one pipeline call site (the control-channel dial), so no other subsystem shares these pooled connections.ILibAsyncSocket_Disconnectsynchronously invalidates the socket (internalSocket = ~0), so the subsequentPipelineRequesttakes the reconnect path (SOCK == NULL || IsFree) rather than racing the teardown.meshServer(the address passed toPipelineRequest); proxy config is applied to the request token afterwards.Testing
tls=upor success) instead of the instantelapsedMs=0failure loop.🤖 Generated with Claude Code
Summary by CodeRabbit