diff --git a/include/tscpp/api/Plugin.h b/include/tscpp/api/Plugin.h index 2f57352cbe0..f37b8050e17 100644 --- a/include/tscpp/api/Plugin.h +++ b/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 */ diff --git a/include/tscpp/api/TransactionPlugin.h b/include/tscpp/api/TransactionPlugin.h index b34fba01f8b..ce3f1ca4419 100644 --- a/include/tscpp/api/TransactionPlugin.h +++ b/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 diff --git a/src/tscpp/api/GlobalPlugin.cc b/src/tscpp/api/GlobalPlugin.cc index b1be2302a1f..8e5f05c2e7e 100644 --- a/src/tscpp/api/GlobalPlugin.cc +++ b/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()); diff --git a/src/tscpp/api/utils_internal.cc b/src/tscpp/api/utils_internal.cc index 7cb86e0941b..61f90446aca 100644 --- a/src/tscpp/api/utils_internal.cc +++ b/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(plugin); + std::shared_ptr 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 &plugins = utils::internal::getTransactionPlugins(transaction); for (auto plugin : plugins) { - std::shared_ptr 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;