Fixes crashes around OCSP with FetchSM

iocore/net/OCSPStapling.cc

@@ -288,12 +288,10 @@ class HTTPRequest : public Continuation
     if (event == TS_EVENT_IMMEDIATE) {
       this->fetch();
     } else {
-      auto fsm = reinterpret_cast<FetchSM *>(e);
-      auto req = reinterpret_cast<HTTPRequest *>(fsm->ext_get_user_data());
       if (event == TS_FETCH_EVENT_EXT_BODY_DONE) {
-        req->set_done();
+        this->set_done();
       } else if (event == TS_EVENT_ERROR) {
-        req->set_error();
+        this->set_error();
       }
     }
 
@@ -310,7 +308,6 @@ class HTTPRequest : public Continuation
     sin.sin_port        = 65535;
 
     this->_fsm = FetchSMAllocator.alloc();
-    this->_fsm->ext_set_user_data(this);
     if (use_post) {
       this->_fsm->ext_init(this, "POST", uri, "HTTP/1.1", reinterpret_cast<sockaddr *>(&sin), 0);
     } else {
@@ -351,35 +348,24 @@ class HTTPRequest : public Continuation
     this->add_header(name, strlen(name), value, strlen(value));
   }
 
-  void
-  fetch()
+  bool
+  is_initiated()
   {
     SCOPED_MUTEX_LOCK(lock, mutex, this_ethread());
-    this->_fsm->ext_launch();
-    this->_fsm->ext_write_data(this->_req_body, this->_req_body_len);
-  }
-
-  void
-  set_done()
-  {
-    this->_result = 1;
-  }
-
-  void
-  set_error()
-  {
-    this->_result = -1;
+    return this->_result != INT_MAX;
   }
 
   bool
   is_done()
   {
-    return this->_result != 0;
+    SCOPED_MUTEX_LOCK(lock, mutex, this_ethread());
+    return this->_result != 0 && this->_result != INT_MAX;
   }
 
   bool
   is_success()
   {
+    SCOPED_MUTEX_LOCK(lock, mutex, this_ethread());
     return this->_result == 1;
   }
 
@@ -396,7 +382,30 @@ class HTTPRequest : public Continuation
   FetchSM *_fsm            = nullptr;
   unsigned char *_req_body = nullptr;
   int _req_body_len        = 0;
-  int _result              = 0;
+  int _result              = INT_MAX;
+
+  void
+  fetch()
+  {
+    SCOPED_MUTEX_LOCK(lock, mutex, this_ethread());
+    this->_result = 0;
+    this->_fsm->ext_launch();
+    this->_fsm->ext_write_data(this->_req_body, this->_req_body_len);
+  }
+
+  void
+  set_done()
+  {
+    SCOPED_MUTEX_LOCK(lock, mutex, this_ethread());
+    this->_result = 1;
+  }
+
+  void
+  set_error()
+  {
+    SCOPED_MUTEX_LOCK(lock, mutex, this_ethread());
+    this->_result = -1;
+  }
 };
 
 TS_OCSP_CERTID *
@@ -1016,13 +1025,21 @@ query_responder(const char *uri, const char *user_agent, TS_OCSP_REQUEST *req, i
   }
 
   // Send request
-  eventProcessor.schedule_imm(&httpreq, ET_NET);
+  Event *e = eventProcessor.schedule_imm(&httpreq, ET_NET);
 
   // Wait until the request completes
   do {
     ink_hrtime_sleep(HRTIME_MSECONDS(1));
   } while (!httpreq.is_done() && (Thread::get_hrtime() < end));
 
+  if (!httpreq.is_done()) {
+    Error("OCSP request was timed out; uri=%s", uri);
+    if (!httpreq.is_initiated()) {
+      Debug("ssl_ocsp", "Request is not initiated yet. Cancelling the event.");
+      e->cancel(&httpreq);
+    }
+  }
+
   if (httpreq.is_success()) {
     // Parse the response
     int len;
@@ -1075,7 +1092,7 @@ stapling_refresh_response(certinfo *cinf, TS_OCSP_RESPONSE **prsp)
 
   *prsp = query_responder(cinf->uri, cinf->user_agent, req, SSLConfigParams::ssl_ocsp_request_timeout);
   if (*prsp == nullptr) {
-    goto done;
+    goto err;
   }
 
   response_status = ASN1_ENUMERATED_get((*prsp)->responseStatus);

src/traffic_server/FetchSM.cc

@@ -46,16 +46,16 @@ FetchSM::cleanUp()
     chunked_handler.clear();
   }
 
+  if (http_vc) {
+    http_vc->do_io_close();
+  }
   free_MIOBuffer(req_buffer);
   free_MIOBuffer(resp_buffer);
   mutex.clear();
   http_parser_clear(&http_parser);
   client_response_hdr.destroy();
   ats_free(client_response);
   cont_mutex.clear();
-  if (http_vc) {
-    http_vc->do_io_close();
-  }
   FetchSMAllocator.free(this);
 }
 
@@ -107,14 +107,13 @@ int
 FetchSM::InvokePlugin(int event, void *data)
 {
   EThread *mythread = this_ethread();
-
-  MUTEX_TAKE_LOCK(contp->mutex, mythread);
-
-  int ret = contp->handleEvent(event, data);
-
-  MUTEX_UNTAKE_LOCK(contp->mutex, mythread);
-
-  return ret;
+  SCOPED_MUTEX_LOCK(lock, mutex, mythread);
+  if (contp) {
+    SCOPED_MUTEX_LOCK(lock2, contp->mutex, mythread);
+    return contp->handleEvent(event, data);
+  } else {
+    return TS_ERROR;
+  }
 }
 
 bool
@@ -734,20 +733,14 @@ FetchSM::ext_read_data(char *buf, size_t len)
 void
 FetchSM::ext_destroy()
 {
+  SCOPED_MUTEX_LOCK(lock, mutex, this_ethread());
+
   contp = nullptr;
 
   if (recursion) {
     return;
   }
 
-  if (fetch_flags & TS_FETCH_FLAGS_NEWLOCK) {
-    MUTEX_TRY_LOCK(lock, mutex, this_ethread());
-    if (!lock.is_locked()) {
-      eventProcessor.schedule_in(this, FETCH_LOCK_RETRY_TIME);
-      return;
-    }
-  }
-
   cleanUp();
 }