[GitHub] [trafficserver] shinrich commented on a change in pull request #7807: Address assert on captive_action

[GitBox <gi...@apache.org>]

shinrich commented on a change in pull request #7807:
URL: https://github.com/apache/trafficserver/pull/7807#discussion_r630606260



##########
File path: proxy/http/HttpCacheSM.cc
##########
@@ -283,6 +283,7 @@ HttpCacheSM::do_cache_open_read(const HttpCacheKey &key)
     return ACTION_RESULT_DONE;
   } else {
     ink_assert(pending_action != nullptr || write_locked == true);
+    captive_action.cancelled = false; // Make sure not cancelled before we hand it out
     return &captive_action;

Review comment:
       No, the captive action is the fixed storage for the action that the HttpCacheSM functions return.  
   
   If you look in HttpSM, you'll see 
   ```
     pending_action =
       c_sm->open_write(&key, s_url, &t_state.hdr_info.client_request, object_read_info,
                        static_cast<time_t>((t_state.cache_control.pin_in_cache_for < 0) ? 0 : t_state.cache_control.pin_in_cache_for),
                        retry, allow_multiple);
   ```
   So the HttpSM's pending_action is pointing at the HttpCacheSM captive_action.  If for some reason, the HttpSM no longer wants to deal with the HttpCacheSM result, it can cancel the captive_action that it's pending action is pointing at.  
   
   The HttpCacheSM's pending action is returned from the scheduling subsystem and may even be null.  So when the HttpCacheSM is cleaning up, it should be canceling the action it's pending_action is pointing at.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org