Fix cache retry assert on ServerAddrSet#12972
Merged
bneradt merged 1 commit intoapache:masterfrom Mar 16, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an ATS crash/assertion when a TSHttpTxnServerAddrSet()-driven origin retry re-enters the cache-miss path while the transaction already holds a cache write lock, and adds an integration test to cover the cached retry case (issue #12971).
Changes:
- Preserve an existing cache write lock when re-entering
HandleCacheOpenReadMiss()during origin retry. - Extend the existing
TSHttpTxnServerAddrSetretry gold test to run both with cache disabled and enabled, and assert no abort/assert output in the cached run.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/proxy/http/HttpTransact.cc |
Avoids re-preparing cache writes (and the redirect-only assert path) when a retry re-enters cache-miss handling with an existing write lock. |
tests/gold_tests/pluginTest/tsapi/test_TSHttpTxnServerAddrSet_retry.test.py |
Adds a second test run with enable_cache=True and validates ATS does not abort/assert during the retry flow. |
You can also share your feedback on Copilot code review. Take the survey.
bryancall
previously approved these changes
Mar 13, 2026
A TSHttpTxnServerAddrSet retry can re-enter the cache miss path after ATS already holds a cache write lock for the same request. The redirect-only prepared-write reuse logic then asserts because this is not actually a redirect. Preserve the existing cache write lock and retry context when the retry returns through HandleCacheOpenReadMiss. Add an autest that fails on the unpatched code with the redirect_in_process assertion and passes with this fix. Fixes: apache#12971
bryancall
approved these changes
Mar 16, 2026
Contributor
|
Cherry-picked to 10.2.x |
cmcfarlen
pushed a commit
that referenced
this pull request
Mar 18, 2026
A TSHttpTxnServerAddrSet retry can re-enter the cache miss path after ATS already holds a cache write lock for the same request. The redirect-only prepared-write reuse logic then asserts because this is not actually a redirect. Preserve the existing cache write lock and retry context when the retry returns through HandleCacheOpenReadMiss. Add an autest that fails on the unpatched code with the redirect_in_process assertion and passes with this fix. Fixes: #12971 (cherry picked from commit bf909ed)
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.
A TSHttpTxnServerAddrSet retry can re-enter the cache miss path after ATS already holds a cache write lock for the same request. The redirect-only prepared-write reuse logic then asserts because this is not actually a redirect.
Preserve the existing cache write lock and retry context when the retry returns through HandleCacheOpenReadMiss. Add an autest that fails on the unpatched code with the redirect_in_process assertion and passes with this fix.
Fixes: #12971