Unify plugins hook dispatch

iocore/cache/test/stub.cc

@@ -39,7 +39,7 @@ APIHooks::append(INKContInternal *cont)
 }
 
 int
-APIHook::invoke(int, void *)
+APIHook::invoke(int, void *) const
 {
   ink_assert(false);
   return 0;
@@ -53,21 +53,29 @@ APIHook::next() const
 }
 
 APIHook *
-APIHooks::get() const
+APIHooks::head() const
 {
   return nullptr;
 }
 
 void
-APIHooks::prepend(INKContInternal *cont)
+APIHooks::clear()
 {
 }
 
+HttpHookState::HttpHookState() {}
+
 void
-APIHooks::clear()
+HttpHookState::init(TSHttpHookID id, HttpAPIHooks const *global, HttpAPIHooks const *ssn, HttpAPIHooks const *txn)
 {
 }
 
+APIHook const *
+HttpHookState::getNext()
+{
+  return nullptr;
+}
+
 void
 ConfigUpdateCbTable::invoke(const char * /* name ATS_UNUSED */)
 {

iocore/net/libinknet_stub.cc

@@ -86,7 +86,7 @@ Log::trace_out(sockaddr const *, unsigned short, char const *, ...)
 
 #include "InkAPIInternal.h"
 int
-APIHook::invoke(int, void *)
+APIHook::invoke(int, void *) const
 {
   ink_assert(false);
   return 0;
@@ -100,7 +100,14 @@ APIHook::next() const
 }
 
 APIHook *
-APIHooks::get() const
+APIHook::prev() const
+{
+  ink_assert(false);
+  return nullptr;
+}
+
+APIHook *
+APIHooks::head() const
 {
   ink_assert(false);
   return nullptr;

proxy/InkAPIInternal.h

@@ -31,6 +31,7 @@
 #include "ProxyConfig.h"
 #include "P_Cache.h"
 #include "I_Tasks.h"
+#include "Plugin.h"
 
 #include "ts/InkAPIPrivateIOCore.h"
 #include "ts/experimental.h"
@@ -114,19 +115,22 @@ class APIHook
 {
 public:
   INKContInternal *m_cont;
-  int invoke(int event, void *edata);
+  int invoke(int event, void *edata) const;
   APIHook *next() const;
+  APIHook *prev() const;
   LINK(APIHook, m_link);
 };
 
 /// A collection of API hooks.
 class APIHooks
 {
 public:
-  void prepend(INKContInternal *cont);
   void append(INKContInternal *cont);
-  APIHook *get() const;
+  /// Get the first hook.
+  APIHook *head() const;
+  /// Remove all hooks.
   void clear();
+  /// Check if there are no hooks.
   bool is_empty() const;
 
 private:
@@ -159,8 +163,6 @@ class FeatureAPIHooks
 
   /// Remove all hooks.
   void clear();
-  /// Add the hook @a cont to the front of the hooks for @a id.
-  void prepend(ID id, INKContInternal *cont);
   /// Add the hook @a cont to the end of the hooks for @a id.
   void append(ID id, INKContInternal *cont);
   /// Get the list of hooks for @a id.
@@ -181,8 +183,11 @@ class FeatureAPIHooks
   /// @return @c true if any hooks of type @a id are present.
   bool has_hooks_for(ID id) const;
 
+  /// Get a pointer to the set of hooks for a specific hook @id
+  APIHooks const *operator[](ID id) const;
+
 private:
-  bool hooks_p = false; ///< Flag for (not) empty container.
+  bool m_hooks_p = false; ///< Flag for (not) empty container.
   /// The array of hooks lists.
   APIHooks m_hooks[N];
 };
@@ -198,28 +203,18 @@ template <typename ID, int N>
 void
 FeatureAPIHooks<ID, N>::clear()
 {
-  for (int i = 0; i < N; ++i) {
-    m_hooks[i].clear();
-  }
-  hooks_p = false;
-}
-
-template <typename ID, int N>
-void
-FeatureAPIHooks<ID, N>::prepend(ID id, INKContInternal *cont)
-{
-  if (likely(is_valid(id))) {
-    hooks_p = true;
-    m_hooks[id].prepend(cont);
+  for (auto &h : m_hooks) {
+    h.clear();
   }
+  m_hooks_p = false;
 }
 
 template <typename ID, int N>
 void
 FeatureAPIHooks<ID, N>::append(ID id, INKContInternal *cont)
 {
-  if (likely(is_valid(id))) {
-    hooks_p = true;
+  if (is_valid(id)) {
+    m_hooks_p = true;
     m_hooks[id].append(cont);
   }
 }
@@ -228,7 +223,12 @@ template <typename ID, int N>
 APIHook *
 FeatureAPIHooks<ID, N>::get(ID id) const
 {
-  return likely(is_valid(id)) ? m_hooks[id].get() : nullptr;
+  return likely(is_valid(id)) ? m_hooks[id].head() : nullptr;
+}
+
+template <typename ID, int N> APIHooks const *FeatureAPIHooks<ID, N>::operator[](ID id) const
+{
+  return likely(is_valid(id)) ? &(m_hooks[id]) : nullptr;
 }
 
 template <typename ID, int N>
@@ -244,7 +244,7 @@ template <typename ID, int N>
 bool
 FeatureAPIHooks<ID, N>::has_hooks() const
 {
-  return hooks_p;
+  return m_hooks_p;
 }
 
 template <typename ID, int N>
@@ -330,6 +330,59 @@ class ConfigUpdateCbTable
   std::unordered_map<std::string, INKContInternal *> cb_table;
 };
 
+class HttpHookState
+{
+public:
+  /// Scope tags for interacting with a live instance.
+  enum ScopeTag { GLOBAL, SSN, TXN };
+
+  /// Default Constructor
+  HttpHookState();
+
+  /// Initialize the hook state to track up to 3 sources of hooks.
+  /// The argument order to this method is used to break priority ties (callbacks from earlier args are invoked earlier)
+  /// The order in terms of @a ScopeTag is GLOBAL, SESSION, TRANSACTION.
+  void init(TSHttpHookID id, HttpAPIHooks const *global, HttpAPIHooks const *ssn = nullptr, HttpAPIHooks const *txn = nullptr);
+
+  /// Select a hook for invocation and advance the state to the next valid hook
+  /// @return nullptr if no current hook.
+  APIHook const *getNext();
+
+  /// Get the hook ID
+  TSHttpHookID id() const;
+
+  /// Temporary function to return true. Later will be used to decide if a plugin is enabled for the hooks
+  bool is_enabled();
+
+protected:
+  /// Track the state of one scope of hooks.
+  struct Scope {
+    APIHook const *_c; ///< Current hook (candidate for invocation).
+    APIHook const *_p; ///< Previous hook (already invoked).
+
+    /// Initialize the scope.
+    void init(HttpAPIHooks const *scope, TSHttpHookID id);
+    /// Clear the scope.
+    void clear();
+    /// Return the current candidate.
+    APIHook const *candidate();
+    /// Advance state to the next hook.
+    void operator++();
+  };
+
+private:
+  TSHttpHookID _id;
+  Scope _global; ///< Chain from global hooks.
+  Scope _ssn;    ///< Chain from session hooks.
+  Scope _txn;    ///< Chain from transaction hooks.
+};
+
+inline TSHttpHookID
+HttpHookState::id() const
+{
+  return _id;
+}
+
 void api_init();
 
 extern HttpAPIHooks *http_global_hooks;

proxy/ProxySession.cc

@@ -77,12 +77,6 @@ static const TSEvent eventmap[TS_HTTP_LAST_HOOK + 1] = {
   TS_EVENT_NONE,                       // TS_HTTP_LAST_HOOK
 };
 
-static bool
-is_valid_hook(TSHttpHookID hookid)
-{
-  return (hookid >= 0) && (hookid < TS_HTTP_LAST_HOOK);
-}
-
 void
 ProxySession::free()
 {
@@ -107,34 +101,28 @@ ProxySession::state_api_callout(int event, void *data)
   case EVENT_NONE:
   case EVENT_INTERVAL:
   case TS_EVENT_HTTP_CONTINUE:
-    if (likely(is_valid_hook(this->api_hookid))) {
-      if (this->api_current == nullptr && this->api_scope == API_HOOK_SCOPE_GLOBAL) {
-        this->api_current = http_global_hooks->get(this->api_hookid);
-        this->api_scope   = API_HOOK_SCOPE_LOCAL;
-      }
+    if (nullptr == cur_hook) {
+      /// Get the next hook to invoke from HttpHookState
+      cur_hook = hook_state.getNext();
+    }
+    if (nullptr != cur_hook) {
+      APIHook const *hook = cur_hook;
 
-      if (this->api_current == nullptr && this->api_scope == API_HOOK_SCOPE_LOCAL) {
-        this->api_current = ssn_hook_get(this->api_hookid);
-        this->api_scope   = API_HOOK_SCOPE_NONE;
-      }
+      MUTEX_TRY_LOCK(lock, hook->m_cont->mutex, mutex->thread_holding);
 
-      if (this->api_current) {
-        APIHook *hook = this->api_current;
-
-        MUTEX_TRY_LOCK(lock, hook->m_cont->mutex, mutex->thread_holding);
-        // Have a mutex but did't get the lock, reschedule
-        if (!lock.is_locked()) {
-          SET_HANDLER(&ProxySession::state_api_callout);
-          if (!schedule_event) { // Don't bother to schedule is there is already one out.
-            schedule_event = mutex->thread_holding->schedule_in(this, HRTIME_MSECONDS(10));
-          }
-          return 0;
+      // Have a mutex but didn't get the lock, reschedule
+      if (!lock.is_locked()) {
+        SET_HANDLER(&ProxySession::state_api_callout);
+        if (!schedule_event) { // Don't bother if there is already one
+          schedule_event = mutex->thread_holding->schedule_in(this, HRTIME_MSECONDS(10));
         }
-
-        this->api_current = this->api_current->next();
-        hook->invoke(eventmap[this->api_hookid], this);
         return 0;
       }
+
+      cur_hook = nullptr; // mark current callback at dispatched.
+      hook->invoke(eventmap[hook_state.id()], this);
+
+      return 0;
     }
 
     handle_api_return(event);
@@ -156,12 +144,10 @@ void
 ProxySession::do_api_callout(TSHttpHookID id)
 {
   ink_assert(id == TS_HTTP_SSN_START_HOOK || id == TS_HTTP_SSN_CLOSE_HOOK);
-
-  this->api_hookid  = id;
-  this->api_scope   = API_HOOK_SCOPE_GLOBAL;
-  this->api_current = nullptr;
-
-  if (this->has_hooks()) {
+  hook_state.init(id, http_global_hooks, &api_hooks);
+  /// Verify if there is any hook to invoke
+  cur_hook = hook_state.getNext();
+  if (nullptr != cur_hook) {
     SET_HANDLER(&ProxySession::state_api_callout);
     this->state_api_callout(EVENT_NONE, nullptr);
   } else {
@@ -172,13 +158,11 @@ ProxySession::do_api_callout(TSHttpHookID id)
 void
 ProxySession::handle_api_return(int event)
 {
-  TSHttpHookID hookid = this->api_hookid;
+  TSHttpHookID hookid = hook_state.id();
 
   SET_HANDLER(&ProxySession::state_api_callout);
 
-  this->api_hookid  = TS_HTTP_LAST_HOOK;
-  this->api_scope   = API_HOOK_SCOPE_NONE;
-  this->api_current = nullptr;
+  cur_hook = nullptr;
 
   switch (hookid) {
   case TS_HTTP_SSN_START_HOOK:
@@ -301,7 +285,7 @@ ProxySession::get_server_session() const
 TSHttpHookID
 ProxySession::get_hookid() const
 {
-  return api_hookid;
+  return hook_state.id();
 }
 
 void
@@ -353,21 +337,21 @@ ProxySession::get_local_addr()
 }
 
 void
-ProxySession::ssn_hook_append(TSHttpHookID id, INKContInternal *cont)
+ProxySession::hook_add(TSHttpHookID id, INKContInternal *cont)
 {
   this->api_hooks.append(id, cont);
 }
 
-void
-ProxySession::ssn_hook_prepend(TSHttpHookID id, INKContInternal *cont)
+APIHook *
+ProxySession::hook_get(TSHttpHookID id) const
 {
-  this->api_hooks.prepend(id, cont);
+  return this->api_hooks.get(id);
 }
 
-APIHook *
-ProxySession::ssn_hook_get(TSHttpHookID id) const
+HttpAPIHooks const *
+ProxySession::feature_hooks() const
 {
-  return this->api_hooks.get(id);
+  return &api_hooks;
 }
 
 bool