Add TXN_CLOSE hook to CPPAPI TransactionPlugin

include/tscpp/api/Plugin.h

@@ -58,7 +58,8 @@ class Plugin : noncopyable
     HOOK_READ_REQUEST_HEADERS,  /**< This hook will be fired after the request is read. */
     HOOK_READ_CACHE_HEADERS,    /**< This hook will be fired after the CACHE hdrs. */
     HOOK_CACHE_LOOKUP_COMPLETE, /**< This hook will be fired after cache lookup complete. */
-    HOOK_SELECT_ALT             /**< This hook will be fired after select alt. */
+    HOOK_TXN_CLOSE, /**< This hook will be fired after send response headers, only for TransactionPlugins::registerHook()!. */
+    HOOK_SELECT_ALT /**< This hook will be fired after select alt. */
   };
 
   /**
@@ -142,6 +143,15 @@ class Plugin : noncopyable
     transaction.resume();
   };
 
+  /**
+   * This method must be implemented when you hook HOOK_TXN_CLOSE
+   */
+  virtual void
+  handleTxnClose(Transaction &transaction)
+  {
+    transaction.resume();
+  };
+
   /**
    * This method must be implemented when you hook HOOK_SELECT_ALT
    */

include/tscpp/api/TransactionPlugin.h

@@ -93,6 +93,10 @@ class TransactionPlugin : public Plugin
    *  see HookType and Plugin for the correspond HookTypes and callback methods. If you fail to implement the
    *  callback, a default implementation will be used that will only resume the Transaction.
    *
+   * \note For automatic destruction, you must either register dynamically allocated instances of
+   *  classes derived from this class with the the corresponding Transaction object (using
+   *  Transaction::addPlugin()), or register HOOK_TXN_CLOSE (but not both).
+   *
    * @param HookType the type of hook you wish to register
    * @see HookType
    * @see Plugin

src/tscpp/api/GlobalPlugin.cc

@@ -87,6 +87,7 @@ GlobalPlugin::~GlobalPlugin()
 void
 GlobalPlugin::registerHook(Plugin::HookType hook_type)
 {
+  assert(hook_type != Plugin::HOOK_TXN_CLOSE);
   TSHttpHookID hook_id = utils::internal::convertInternalHookToTsHook(hook_type);
   TSHttpHookAdd(hook_id, state_->cont_);
   LOG_DEBUG("Registered global plugin %p for hook %s", this, HOOK_TYPE_STRINGS[hook_type].c_str());

src/tscpp/api/utils_internal.cc

@@ -49,6 +49,25 @@ resetTransactionHandles(Transaction &transaction, TSEvent event)
   return;
 }
 
+void
+cleanupTransaction(Transaction &transaction, TSHttpTxn ats_txn_handle)
+{
+  delete &transaction;
+  // reset the txn arg to prevent use-after-free
+  TSUserArgSet(ats_txn_handle, TRANSACTION_STORAGE_INDEX, nullptr);
+}
+
+void
+cleanupTransactionPlugin(Plugin *plugin)
+{
+  TransactionPlugin *transaction_plugin = static_cast<TransactionPlugin *>(plugin);
+  std::shared_ptr<Mutex> trans_mutex    = utils::internal::getTransactionPluginMutex(*transaction_plugin);
+  LOG_DEBUG("Locking TransactionPlugin mutex to delete transaction plugin at %p", transaction_plugin);
+  trans_mutex->lock();
+  delete transaction_plugin;
+  trans_mutex->unlock();
+}
+
 int
 handleTransactionEvents(TSCont cont, TSEvent event, void *edata)
 {
@@ -77,14 +96,9 @@ handleTransactionEvents(TSCont cont, TSEvent event, void *edata)
     resetTransactionHandles(transaction, event);
     const std::list<TransactionPlugin *> &plugins = utils::internal::getTransactionPlugins(transaction);
     for (auto plugin : plugins) {
-      std::shared_ptr<Mutex> trans_mutex = utils::internal::getTransactionPluginMutex(*plugin);
-      LOG_DEBUG("Locking TransactionPlugin mutex to delete transaction plugin at %p", plugin);
-      trans_mutex->lock();
-      LOG_DEBUG("Locked Mutex...Deleting transaction plugin at %p", plugin);
-      delete plugin;
-      trans_mutex->unlock();
+      cleanupTransactionPlugin(plugin);
     }
-    delete &transaction;
+    cleanupTransaction(transaction, ats_txn_handle);
   } break;
   default:
     assert(false); /* we should never get here */
@@ -141,6 +155,15 @@ void inline invokePluginForEvent(Plugin *plugin, TSHttpTxn ats_txn_handle, TSEve
   case TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE:
     plugin->handleReadCacheLookupComplete(transaction);
     break;
+  case TS_EVENT_HTTP_TXN_CLOSE:
+    if (plugin) {
+      plugin->handleTxnClose(transaction);
+      cleanupTransactionPlugin(plugin);
+    } else {
+      LOG_ERROR("stray event TS_EVENT_HTTP_TXN_CLOSE, no transaction plugin to handle it!");
+    }
+    cleanupTransaction(transaction, ats_txn_handle);
+    break;
   default:
     assert(false); /* we should never get here */
     break;
@@ -191,6 +214,8 @@ utils::internal::convertInternalHookToTsHook(Plugin::HookType hooktype)
     return TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK;
   case Plugin::HOOK_SELECT_ALT:
     return TS_HTTP_SELECT_ALT_HOOK;
+  case Plugin::HOOK_TXN_CLOSE:
+    return TS_HTTP_TXN_CLOSE_HOOK;
   default:
     assert(false); // shouldn't happen, let's catch it early
     break;