JosiahWI commented on code in PR #13309:
URL: https://github.com/apache/trafficserver/pull/13309#discussion_r3455921037
##########
src/proxy/http/HttpTransact.cc:
##########
@@ -3565,16 +3565,18 @@
HttpTransact::set_cache_prepare_write_action_for_new_request(State *s)
// This method must be called no more than one time per request. It should
// not be called for non-cacheable requests.
if (s->cache_info.write_lock_state == CacheWriteLock_t::SUCCESS) {
- // If and only if this is a redirected request, we may have already
- // prepared a cache write (during the handling of the previous request
- // which got the 3xx response) and can safely re-use it. Otherwise, we
- // risk storing the response under the wrong cache key. This is a release
- // assert because the correct behavior would be to prepare a new write,
- // but we can't do that because we failed to release the lock. To recover
- // we would have to tell the state machine to abort its write, and we
- // don't have a state for that.
+ // If and only if this is a redirected request, we may have already
prepared
+ // a cache write during the handling of the previous request and can safely
+ // re-use it while the prepared write VC is still available. Otherwise, we
+ // risk storing the response under the wrong cache key.
ink_release_assert(s->redirect_info.redirect_in_process);
- s->cache_info.action = CacheAction_t::WRITE;
+ if (s->state_machine->get_cache_sm().cache_write_vc != nullptr) {
+ s->cache_info.action = CacheAction_t::WRITE;
+ } else {
Review Comment:
I think it would be better to release the cache lock and reset
`write_lock_state` at the point where we close the cache VConnection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
