okhttp: Fix bidirectional keep-alive causing spurious GO_AWAY (1.77.x backport) #12506
+30
−5
Conversation
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
When bidirectional keep-alive is enabled (both client and server sending keep-alive pings), the server incorrectly validates ping acknowledgments (ACKs) sent in response to server-initiated pings. This causes the KeepAliveEnforcer strike counter to increment for legitimate protocol responses, eventually triggering a GO_AWAY with ENHANCE_YOUR_CALM. Move the keepAliveEnforcer.pingAcceptable() check inside the !ack conditional block, so only client-initiated ping requests are validated. Ping ACKs now bypass enforcement entirely, enabling bidirectional keep-alive to work correctly. Fixes grpc#12417
kannanjgithub
approved these changes
Nov 13, 2025
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.
Problem
When bidirectional keep-alive is enabled (both client and server sending keep-alive pings), the server incorrectly terminates connections with a
GO_AWAYframe even when the client is compliant with the server's keep-alive policies. This occurs because theKeepAliveEnforcervalidates all incoming ping frames—both client-initiated ping requests and ping acknowledgments (ACKs) sent in response to server-initiated pings.In a bidirectional keep-alive scenario, the client sends ACKs back to the server for each server-initiated ping. These ACKs are legitimate protocol responses, not abusive client behavior. However, the enforcer treats them as strike-worthy events, causing the strike counter to increment until
MAX_PING_STRIKESis exceeded, triggering a connection shutdown withENHANCE_YOUR_CALM.This breaks bidirectional keep-alive entirely—a legitimate use case for detecting network failures from both endpoints.
Solution
The keep-alive enforcement logic now only validates client-initiated ping requests (
ack=false), not ping acknowledgments (ack=true). This allows servers and clients to exchange keep-alive pings independently without either side's pings counting against the other's enforcement limits.The fix moves the
keepAliveEnforcer.pingAcceptable()check inside the!ackconditional block inokhttp/src/main/java/io/grpc/okhttp/OkHttpServerTransport.java:954, ensuring ping ACKs bypass validation entirely.Impact
This fix enables bidirectional keep-alive to work correctly. Clients can now respond to server pings without triggering spurious connection shutdowns. Server-side keep-alive enforcement continues to protect against abusive client behavior for actual ping requests.
No breaking changes. This corrects existing behavior to match the expected semantics of the HTTP/2 ping mechanism.
Fixes #12417
Backport of #12502
CC @kssumin