-
Notifications
You must be signed in to change notification settings - Fork 4k
core: Fix inFlightSubStreams counting on retry commit #12649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core: Fix inFlightSubStreams counting on retry commit #12649
Conversation
Under certain retry and deadline timings, RetriableStream could leave the response future incomplete because inFlightSubStreams was not decremented consistently during commit. The change ensures inFlightSubStreams is decremented whenever a scheduled retry is committed, restoring correct close signaling without altering retry semantics.
| final Future<?> retryFuture; | ||
| final boolean retryWasScheduled = scheduledRetry != null; | ||
| if (scheduledRetry != null) { | ||
| retryFuture = scheduledRetry.markCancelled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
markCancelled() may return null if the scheduled retry was cancelled before setFuture() was called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, when the hang occurs I see markCancelled() had returned null causing the code in CommitTask to skip closing the master listener.
|
/gcbrun |
This PR fixes a race condition in RetriableStream where, under certain retry and deadline timings, the response future may never be completed. When a deadline cancellation occurs concurrently with retry commit, inFlightSubStreams may not be decremented, causing `ClientCallImpl.ClientStreamListenerImpl.closed` to never be invoked. As a result, blockingUnaryCall can hang indefinitely. After this change, the inFlightSubStreams counting is consistent whenever a scheduled retry is committed, ensuring the close signal is always delivered. I verified this using the issue reproduction code from the issue reporter, which previously caused blockingUnaryCall to hang and eventually hit a TimeoutException because a while loop never progressed. After this change, running the same reproduction code no longer hangs and continues as expected without timing out. Fixes grpc#12620
This PR fixes a race condition in RetriableStream where, under certain retry and deadline timings, the response future may never be completed.
When a deadline cancellation occurs concurrently with retry commit, inFlightSubStreams may not be decremented, causing
ClientCallImpl.ClientStreamListenerImpl.closedto never be invoked. As a result, blockingUnaryCall can hang indefinitely.After this change, the inFlightSubStreams counting is consistent whenever a scheduled retry is committed, ensuring the close signal is always delivered.
I verified this using the issue reproduction code from the issue reporter, which previously caused blockingUnaryCall to hang and eventually hit a TimeoutException because a while loop never progressed. After this change, running the same reproduction code no longer hangs and continues as expected without timing out.
Fixes #12620