From 122c87012cf1cbd449fd1d7119ee6d9c26d23fdb Mon Sep 17 00:00:00 2001 From: Josiah VanderZee Date: Thu, 24 Apr 2025 17:16:20 -0500 Subject: [PATCH] Reset write lock state to init after closing write This commit mitigates the crash reported in #11700. That crash happens when a redirect is issued on a state machine that has already cached a response and closed the cache write VC. After this patch, the state machine will likely open a new cache VC to cache the response from the origin it was redirected to. We will refer to the original origin as A, and the origin the state machine was redirected to after the response from A as B. We have not yet reproduced this locally - the exact sequence of events that gets the state machine into this state are still unknown. Some things to pay attention to for review: * Normal behavior is to cache B's response under A's URI. In the edge case this patch mitigates, A and B's responses will both be cached, possibly with B's response overwriting A's, or maybe not... this is still untested. * This does a second cache write when normally only one cache write occurs during a state machine's lifetime. Are both writes independent from each other's state, and properly cleaned up to prevent memory leaks? * Are there other places where the write lock should also be reset? * Can the escalate plugin force a redirect to happen after `kill_this()` has been called (we tried to detect this with a release assert, and did not). --- src/proxy/http/HttpSM.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index a54cc7cccc7..46c50b20dcc 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -6460,6 +6460,7 @@ HttpSM::perform_cache_write_action() // Write close deletes the old alternate cache_sm.close_write(); cache_sm.close_read(); + t_state.cache_info.write_lock_state = HttpTransact::CACHE_WL_INIT; break; } @@ -6518,6 +6519,7 @@ HttpSM::issue_cache_update() } // Now close the write which commits the update cache_sm.close_write(); + t_state.cache_info.write_lock_state = HttpTransact::CACHE_WL_INIT; } int