Address assert on captive_action (#7807)

(cherry picked from commit eb765c0)

proxy/http/HttpCacheSM.cc

@@ -101,6 +101,10 @@ HttpCacheSM::state_cache_open_read(int event, void *data)
   ink_assert(captive_action.cancelled == 0);
   pending_action = nullptr;
 
+  if (captive_action.cancelled == 1) {
+    return VC_EVENT_CONT; // SM gave up on us
+  }
+
   switch (event) {
   case CACHE_EVENT_OPEN_READ:
     HTTP_INCREMENT_DYN_STAT(http_current_cache_connections_stat);
@@ -111,10 +115,11 @@ HttpCacheSM::state_cache_open_read(int event, void *data)
     }
     open_read_cb  = true;
     cache_read_vc = static_cast<CacheVConnection *>(data);
-    master_sm->handleEvent(event, data);
+    master_sm->handleEvent(event, &captive_action);
     break;
 
   case CACHE_EVENT_OPEN_READ_FAILED:
+    err_code = reinterpret_cast<intptr_t>(data);
     if ((intptr_t)data == -ECACHE_DOC_BUSY) {
       // Somebody else is writing the object
       if (open_read_tries <= master_sm->t_state.txn_conf->max_cache_open_read_retries) {
@@ -125,12 +130,12 @@ HttpCacheSM::state_cache_open_read(int event, void *data)
         // Give up; the update didn't finish in time
         // HttpSM will inform HttpTransact to 'proxy-only'
         open_read_cb = true;
-        master_sm->handleEvent(event, data);
+        master_sm->handleEvent(event, &captive_action);
       }
     } else {
       // Simple miss in the cache.
       open_read_cb = true;
-      master_sm->handleEvent(event, data);
+      master_sm->handleEvent(event, &captive_action);
     }
     break;
 
@@ -159,7 +164,11 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
 {
   STATE_ENTER(&HttpCacheSM::state_cache_open_write, event);
   ink_assert(captive_action.cancelled == 0);
-  pending_action                = nullptr;
+  pending_action = nullptr;
+
+  if (captive_action.cancelled == 1) {
+    return VC_EVENT_CONT; // SM gave up on us
+  }
   bool read_retry_on_write_fail = false;
 
   switch (event) {
@@ -168,7 +177,7 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
     ink_assert(cache_write_vc == nullptr);
     cache_write_vc = static_cast<CacheVConnection *>(data);
     open_write_cb  = true;
-    master_sm->handleEvent(event, data);
+    master_sm->handleEvent(event, &captive_action);
     break;
 
   case CACHE_EVENT_OPEN_WRITE_FAILED:
@@ -196,8 +205,6 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
     if (read_retry_on_write_fail || open_write_tries <= master_sm->t_state.txn_conf->max_cache_open_write_retries) {
       // Retry open write;
       open_write_cb = false;
-      // reset captive_action since HttpSM cancelled it
-      captive_action.cancelled = 0;
       do_schedule_in();
     } else {
       // The cache is hosed or full or something.
@@ -207,7 +214,8 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
             "done retrying...",
             master_sm->sm_id, open_write_tries);
       open_write_cb = true;
-      master_sm->handleEvent(event, data);
+      err_code      = reinterpret_cast<intptr_t>(data);
+      master_sm->handleEvent(event, &captive_action);
     }
     break;
 
@@ -218,7 +226,7 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
             "falling back to read retry...",
             master_sm->sm_id, open_write_tries);
       open_read_cb = false;
-      master_sm->handleEvent(CACHE_EVENT_OPEN_READ, data);
+      master_sm->handleEvent(CACHE_EVENT_OPEN_READ, &captive_action);
     } else {
       Debug("http_cache",
             "[%" PRId64 "] [state_cache_open_write] cache open write failure %d. "
@@ -266,8 +274,6 @@ HttpCacheSM::do_cache_open_read(const HttpCacheKey &key)
   } else {
     ink_assert(open_read_cb == false);
   }
-  // reset captive_action since HttpSM cancelled it during open read retry
-  captive_action.cancelled = 0;
   // Initialising read-while-write-inprogress flag
   this->readwhilewrite_inprogress = false;
   Action *action_handle = cacheProcessor.open_read(this, &key, this->read_request_hdr, this->http_params, this->read_pin_in_cache);
@@ -283,6 +289,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 = 0; // Make sure not cancelled before we hand it out
     return &captive_action;
   }
 }
@@ -355,7 +362,8 @@ HttpCacheSM::open_write(const HttpCacheKey *key, URL *url, HTTPHdr *request, Cac
   // Changed by YTS Team, yamsat Plugin
   if (open_write_tries > master_sm->redirection_tries &&
       open_write_tries > master_sm->t_state.txn_conf->max_cache_open_write_retries) {
-    master_sm->handleEvent(CACHE_EVENT_OPEN_WRITE_FAILED, (void *)-ECACHE_DOC_BUSY);
+    err_code = -ECACHE_DOC_BUSY;
+    master_sm->handleEvent(CACHE_EVENT_OPEN_WRITE_FAILED, &captive_action);
     return ACTION_RESULT_DONE;
   }
 
@@ -375,6 +383,7 @@ HttpCacheSM::open_write(const HttpCacheKey *key, URL *url, HTTPHdr *request, Cac
     return ACTION_RESULT_DONE;
   } else {
     ink_assert(pending_action != nullptr);
+    captive_action.cancelled = 0; // Make sure not cancelled before we hand it out
     return &captive_action;
   }
 }

proxy/http/HttpCacheSM.h

@@ -183,6 +183,12 @@ class HttpCacheSM : public Continuation
     abort_write();
   }
 
+  inline int
+  get_last_error() const
+  {
+    return err_code;
+  }
+
 private:
   void do_schedule_in();
   Action *do_cache_open_read(const HttpCacheKey &);
@@ -211,4 +217,7 @@ class HttpCacheSM : public Continuation
   // to keep track of multiple cache lookups
   int lookup_max_recursive = 0;
   int current_lookup_level = 0;
+
+  // last error from the cache subsystem
+  int err_code = 0;
 };

proxy/http/HttpSM.cc

@@ -2512,6 +2512,8 @@ HttpSM::state_cache_open_write(int event, void *data)
     ink_release_assert(vc && vc->thread == this_ethread());
   }
 
+  pending_action.clear_if_action_is(reinterpret_cast<Action *>(data));
+
   milestones[TS_MILESTONE_CACHE_OPEN_WRITE_END] = Thread::get_hrtime();
   pending_action                                = nullptr;
 
@@ -2621,6 +2623,8 @@ HttpSM::state_cache_open_read(int event, void *data)
   STATE_ENTER(&HttpSM::state_cache_open_read, event);
   milestones[TS_MILESTONE_CACHE_OPEN_READ_END] = Thread::get_hrtime();
 
+  pending_action.clear_if_action_is(reinterpret_cast<Action *>(data));
+
   ink_assert(server_entry == nullptr);
   ink_assert(t_state.cache_info.object_read == nullptr);
 
@@ -2652,12 +2656,12 @@ HttpSM::state_cache_open_read(int event, void *data)
     pending_action = nullptr;
 
     SMDebug("http", "[%" PRId64 "] cache_open_read - CACHE_EVENT_OPEN_READ_FAILED with %s (%d)", sm_id,
-            InkStrerror(-(intptr_t)data), (int)(intptr_t)data);
+            InkStrerror(-cache_sm.get_last_error()), -cache_sm.get_last_error());
 
     SMDebug("http", "[state_cache_open_read] open read failed.");
     // Inform HttpTransact somebody else is updating the document
     // HttpCacheSM already waited so transact should go ahead.
-    if ((intptr_t)data == -ECACHE_DOC_BUSY) {
+    if (cache_sm.get_last_error() == -ECACHE_DOC_BUSY) {
       t_state.cache_lookup_result = HttpTransact::CACHE_LOOKUP_DOC_BUSY;
     } else {
       t_state.cache_lookup_result = HttpTransact::CACHE_LOOKUP_MISS;

proxy/http/HttpSM.h

@@ -197,6 +197,13 @@ class PendingAction
   {
     return pending_action;
   }
+  void
+  clear_if_action_is(Action *current_action)
+  {
+    if (current_action == pending_action) {
+      pending_action = nullptr;
+    }
+  }
   ~PendingAction()
   {
     if (pending_action) {