Add named objects to context.

doc/dev/memory-management.en.rst

@@ -11,38 +11,31 @@
 Memory Mangement
 ****************
 
-While most elements do not require additional memory, this is not always the case. If elements in a
-constellation need to share
+While most elements do not require additional memory, this is not always the case. If additional memory is needed to
+perform a task, this should be allocated in |TxB| memory that has a transaction based lifetime for faster allocation and
+automated cleanup. If elements in a constellation need to share data, the data must be placed in |TxB| managed memory to
+avoid corrouption or crashes on configuration reload.
 
-
-|TxB| provides
-a number of mechanisms for allocating memory efficiently in the context of handling transactions.
-
-The primary reason to use |TxB| memory instead of :code:`malloc` is memory lifetime management.
-Using the internal mechanisms memory that has exactly the lifetime of a configuration or a
-transaction is straightforward. In many cases the code can simply allocate without concern of leaks.
-In addition this restriction also makes the allocation much faster than with :code:`malloc`.
-
-Another reason is avoiding race conditions and collisions. Because configurations can be dynamically
-reloaded, pointers to configuration local memory that are stored statically can go bad or become
-corrupted during reload as the pointers are moved while a configuration is still in use. Use of the
-internal mechanisms ties the memory to a particular configuration or transaction context, avoiding
-this problem.
+|TxB| provides a number of mechanisms for allocating memory efficiently for configuration and transaction
+lifetimes.
 
 =====================
 Configuration Storage
 =====================
 
 The root of memomry management is the configuration instance, represented by an instance of
-:txb:`Config`. While allocating memory for its own uses, directives can request memmory in the
-configuration to be reserved for that directive. This is per directive class, not per directive
-instance. Access to the memory is via the inherited pointer :txb:`Directive::_rtti`.
-
-When the directive is defined with :txb:`Config::define` there is an options structure of type
-:txb:`Directive::Options` that can specify the amount of reserved storage in bytes in the
-:code:`_cfg_store_required` member. If the directive is used, the specified amount of storage is
-reserved. It is accessed by :code:`_rtti->_cfg_store`. This is of type :code:`MemSpan<void>` and
-specifies the reserved memory.
+:txb:`Config`. This contains a memory arena with the same lifetime as the configuration and is used to store
+configuration related data. Elements can also allocate memory in this arena. Such memory has two useful
+properties.
+
+*  The memory lifetime is the same as the configuration instance.
+*  The memory is accessible from any element.
+
+Raw configuration memory can be allocated as described later but this leaves the problem of locating that memory
+again, with the self referential problem of having to know the location of the memory in the configuration to
+find the location. The method :txb:`Config::obtain_named_object` provides a bootstrapping object in configuration.
+This finds or creates an instance of a specific type in configuration memory and associates it with a name. The
+method :txb:`Config::named_object` can be used later to find that object.
 
 The most challenging aspect is finding configuration allocated memory later. If the configuration
 based memory is used per directive instance, this is not a problem - a span can be stored in the

plugin/include/txn_box/Config.h

@@ -117,15 +117,17 @@ class Config
     ~ActiveCaptureScope();
   };
   friend ActiveCaptureScope;
-  ActiveCaptureScope
-  capture_scope(unsigned count, unsigned line_no)
-  {
-    ActiveCaptureScope scope(*this);
-    _active_capture._count = count;
-    _active_capture._line  = line_no;
-    _active_capture._ref_p = false;
-    return scope;
-  }
+
+  /** Preserve the current capture group state.
+   *
+   * @param count Number of capture groups in the new state.
+   * @param line_no Configuration line number that required the change.
+   * @return A cached scope object.
+   *
+   * The current state is preserved in the returned object which, when destructed,
+   * restores the previous state.
+   */
+  ActiveCaptureScope capture_scope(unsigned count, unsigned line_no);
 
   /** Local extractor table.
    *
@@ -389,15 +391,13 @@ class Config
    * This is used when a directive needs to be available under an alternative name. All of the arguments
    * are pulled from standard class members except the key (directive name).
    */
-  template <typename D>
-  static swoc::Errata
-  define(swoc::TextView name);
+  template <typename D> static swoc::Errata define(swoc::TextView name);
 
   /** Allocate storage in @a this.
    *
    * @param n Size in bytes.
    * @param align Memory alignment.
-   * @return The allocated span.
+   * @return The allocated memory.
    */
   swoc::MemSpan<void> allocate_cfg_storage(size_t n, size_t align = 1);
 

plugin/include/txn_box/Context.h

@@ -15,6 +15,7 @@
 
 #include <swoc/MemArena.h>
 #include <swoc/Errata.h>
+#include <swoc/IntrusiveHashMap.h>
 
 #include "txn_box/common.h"
 #include "txn_box/Rxp.h"
@@ -37,12 +38,6 @@ class Context
 {
   using self_type = Context;
   friend class Config;
-  /// Cleanup functor for an inverted arena.
-  /// @internal Because the arena is inverted, calling the destructor will clean up everything.
-  /// For this reason @c delete must @b not be called (that will double free).
-  struct ArenaDestructor {
-    void operator()(swoc::MemArena *arena);
-  };
 
 public:
   /// Construct based a specific configuration.
@@ -147,7 +142,32 @@ class Context
    *
    * @see mark_for_cleanup
    */
-  template <typename T> swoc::MemSpan<T> alloc_span(unsigned count);
+  template <typename T> swoc::MemSpan<T> alloc_span(unsigned count, size_t align = 1);
+
+  /** Find or allocate an instance of @a T in configuration storage.
+   *
+   * @tparam T Type of object.
+   * @tparam Args Arguments to @a T constructor.
+   * @param name Name of object.
+   * @return A pointer to an initialized instance of @a T in configuration storage.
+   *
+   * Looks for the named object and if found returns it. If the name is not found, a @a T is
+   * allocated and constructed with the arguments after @a name forwarded.
+   *
+   * @note This should only be called during configuration loading.
+   */
+  template < typename T, typename ... Args > T * obtain_named_object(swoc::TextView name, Args && ... args);
+
+  /** Find named object.
+   *
+   * @tparam T Expected type of object.
+   * @param name Name of the object.
+   * @return A pointer to the object, or @c nullptr if not found.
+   *
+   * @note The caller is presumed to know that @a T is correct, no checks are done. This is purely a convenience to
+   * avoid excessive casting.
+   */
+  template < typename T > T * named_object(swoc::TextView name);
 
   /** Require @a n bytes of transient buffer.
    *
@@ -515,6 +535,13 @@ class Context
     swoc::MemSpan<void> _storage;
   };
 
+  /// Cleanup functor for an inverted arena.
+  /// @internal Because the arena is inverted, calling the destructor will clean up everything.
+  /// For this reason @c delete must @b not be called (that will double free).
+  struct ArenaDestructor {
+    void operator()(swoc::MemArena *arena);
+  };
+
   /// Transaction local storage.
   /// This is a pointer so that the arena can be inverted to minimize allocations.
   std::unique_ptr<swoc::MemArena, ArenaDestructor> _arena;
@@ -567,20 +594,9 @@ class Context
     /// Linkage for @c IntrusiveHashMap.
     struct Linkage : public swoc::IntrusiveLinkage<self_type, &self_type::_next, &self_type::_prev> {
       static swoc::TextView
-      key_of(self_type *self)
-      {
-        return self->_name;
-      }
-      static auto
-      hash_of(swoc::TextView const &text) -> decltype(Hash_Func(text))
-      {
-        return Hash_Func(text);
-      }
-      static bool
-      equal(swoc::TextView const &lhs, swoc::TextView const &rhs)
-      {
-        return lhs == rhs;
-      }
+      key_of(self_type *self) { return self->_name; }
+      static auto hash_of(swoc::TextView const &text) -> decltype(Hash_Func(text)){ return Hash_Func(text); }
+      static bool equal(swoc::TextView const &lhs, swoc::TextView const &rhs) { return lhs == rhs; }
     };
 
     TxnVar(swoc::TextView const &name, Feature const &value) : _name(name), _value(value) {}
@@ -589,6 +605,30 @@ class Context
   using TxnVariables = swoc::IntrusiveHashMap<TxnVar::Linkage>;
   TxnVariables _txn_vars; ///< Variables for the transaction.
 
+  /// Internal named object local to the context.
+  struct NamedObject {
+    using self_type = NamedObject; ///< Self reference type.
+    static constexpr std::hash<std::string_view> Hash_Func{};
+
+    swoc::TextView _name;       ///< Name of variable.
+    swoc::MemSpan<void> _span;  ///< Object memroy.
+    self_type *_next = nullptr; ///< Intrusive link.
+    self_type *_prev = nullptr; ///< Intrusive link.
+
+    /// Linkage for @c IntrusiveHashMap.
+    struct Linkage : public swoc::IntrusiveLinkage<self_type, &self_type::_next, &self_type::_prev> {
+      static swoc::TextView
+      key_of(self_type *self) { return self->_name; }
+      static auto hash_of(swoc::TextView const &text) -> decltype(Hash_Func(text)){ return Hash_Func(text); }
+      static bool equal(swoc::TextView const &lhs, swoc::TextView const &rhs) { return lhs == rhs; }
+    };
+
+    NamedObject(swoc::TextView const &name, swoc::MemSpan<void> span) : _name(name), _span(span) {}
+  };
+
+  using NamedObjects = swoc::IntrusiveHashMap<NamedObject::Linkage>;
+  NamedObjects _named_objects; ///< Conext local data objects.
+
   /// Flag for continuing invoking directives.
   bool _terminal_p = false;
 
@@ -639,10 +679,10 @@ Context::mark_for_cleanup(T* ptr, void (*cleaner)(T*))
 
 template <typename T>
 swoc::MemSpan<T>
-Context::alloc_span(unsigned int count)
+Context::alloc_span(unsigned count, size_t align)
 {
   this->commit_transient();
-  return _arena->alloc(sizeof(T) * count).rebind<T>();
+  return _arena->alloc(sizeof(T) * count, align).rebind<T>();
 }
 
 inline swoc::MemSpan<void>
@@ -754,3 +794,25 @@ Context::inbound_ssn()
 {
   return _txn.inbound_ssn();
 }
+
+template <typename T, typename... Args>
+T *
+Context::obtain_named_object(swoc::TextView name, Args&&... args)
+{
+  auto spot = _named_objects.find(name);
+  if (spot != _named_objects.end()) {
+    return static_cast<T*>(spot->_span.data());
+  }
+
+  auto span = this->alloc_span<T>(1, alignof(T));
+  _named_objects.insert(this->make<NamedObject>(name, span));
+  return new (span.data()) T(std::forward<Args>(args)...);
+}
+
+template <typename T>
+T *
+Context::named_object(swoc::TextView name)
+{
+  auto spot = _named_objects.find(name);
+  return spot != _named_objects.end() ? static_cast<T*>(spot->_span.data()) : nullptr;
+}

plugin/src/Config.cc

@@ -813,6 +813,16 @@ Config::feature_scope(ActiveType const &ex_type)
   return scope;
 }
 
+Config::ActiveCaptureScope
+Config::capture_scope(unsigned int count, unsigned int line_no)
+{
+  ActiveCaptureScope scope(*this);
+  _active_capture._count = count;
+  _active_capture._line  = line_no;
+  _active_capture._ref_p = false;
+  return scope;
+}
+
 Config::ActiveFeatureScope::~ActiveFeatureScope()
 {
   if (_cfg) {

plugin/src/Machinery.cc

@@ -2626,11 +2626,6 @@ class Do_proxy_reply : public Directive
   using self_type  = Do_proxy_reply; ///< Self reference type.
   using super_type = Directive;      ///< Parent type.
 
-  /// Per configuration storage.
-  struct CfgInfo {
-    ReservedSpan _ctx_span; ///< Reserved span for @c CtxInfo.
-  };
-
   /// Per context information.
   /// This is what is stored in the span @c CfgInfo::_ctx_span
   struct CtxInfo {
@@ -2643,7 +2638,7 @@ class Do_proxy_reply : public Directive
   inline static const std::string REASON_KEY = "reason";      ///< Key for reason value.
   inline static const std::string BODY_KEY   = "body";        ///< Key for body.
 
-  static const HookMask HOOKS; ///< Valid hooks for directive.
+  static inline const HookMask HOOKS{MaskFor({Hook::CREQ, Hook::PRE_REMAP, Hook::REMAP})}; ///< Valid hooks for directive.
 
   /// Need to do fixups on a later hook.
   static constexpr Hook FIXUP_HOOK = Hook::PRSP;
@@ -2662,9 +2657,7 @@ class Do_proxy_reply : public Directive
   static Rv<Handle> load(Config &cfg, CfgStaticData const *, YAML::Node drtv_node, swoc::TextView const &name,
                          swoc::TextView const &arg, YAML::Node key_value);
 
-  /// Configuration level initialization.
-  static Errata cfg_init(Config &cfg, CfgStaticData const *rtti);
-
+  static Errata cfg_init(Config &cfg, CfgStaticData const *);
 protected:
   using index_type = FeatureGroup::index_type;
 
@@ -2683,25 +2676,17 @@ class Do_proxy_reply : public Directive
   Errata fixup(Context &ctx);
 };
 
-const HookMask Do_proxy_reply::HOOKS{MaskFor({Hook::CREQ, Hook::PRE_REMAP, Hook::REMAP})};
-
 Errata
 Do_proxy_reply::cfg_init(Config &cfg, CfgStaticData const *)
 {
-  auto cfg_info = cfg.obtain_named_object<CfgInfo>(KEY);
-  // Only one proxy_reply can be effective per transaction, therefore shared state per context is best.
-  cfg_info->_ctx_span = cfg.reserve_ctx_storage(sizeof(CtxInfo));
   cfg.reserve_slot(FIXUP_HOOK); // needed to fix up "Location" field in proxy response.
   return {};
 }
 
 Errata
 Do_proxy_reply::invoke(Context &ctx)
 {
-  auto cfg_info = ctx.cfg().named_object<CfgInfo>(KEY);
-  if (!cfg_info) { return {}; } // Indicates shutdown is ongoing.
-
-  auto ctx_info = ctx.initialized_storage_for<CtxInfo>(cfg_info->_ctx_span).data();
+  auto ctx_info = ctx.obtain_named_object<CtxInfo>(KEY);
 
   // Is a fix up hook required to set the reason correctly?
   bool need_hook_p = false;
@@ -2742,11 +2727,9 @@ Do_proxy_reply::invoke(Context &ctx)
 Errata
 Do_proxy_reply::fixup(Context &ctx)
 {
-  if ( auto cfg_info = ctx.cfg().named_object<CfgInfo>(KEY) ; cfg_info ) {
-    auto ctx_info = ctx.storage_for(cfg_info->_ctx_span).rebind<CtxInfo>().data();
-    auto hdr{ctx.proxy_rsp_hdr()};
-    // Set the reason.
+  if ( auto ctx_info = ctx.named_object<CtxInfo>(KEY) ; ctx_info ) {
     if (!ctx_info->_reason.empty()) {
+      auto hdr{ctx.proxy_rsp_hdr()};
       hdr.reason_set(ctx_info->_reason);
     }
   }
@@ -2846,11 +2829,6 @@ class Do_redirect : public Directive
   using self_type  = Do_redirect; ///< Self reference type.
   using super_type = Directive;   ///< Parent type.
 
-  /// Per configuration storage.
-  struct CfgInfo {
-    ReservedSpan _ctx_span; ///< Reserved span for @c CtxInfo.
-  };
-
   /// Per context information, used for fix up on proxy response hook.
   /// -- doc Do_redirect::CtxInfo
   struct CtxInfo {
@@ -2913,8 +2891,6 @@ const HookMask Do_redirect::HOOKS{MaskFor(Hook::PRE_REMAP, Hook::REMAP)};
 Errata
 Do_redirect::cfg_init(Config &cfg, CfgStaticData const *)
 {
-  auto cfg_info = cfg.obtain_named_object<CfgInfo>(KEY);
-  cfg_info->_ctx_span = cfg.reserve_ctx_storage(sizeof(CtxInfo));
   cfg.reserve_slot(FIXUP_HOOK); // needed to fix up "Location" field in proxy response.
   return {};
 }
@@ -2923,11 +2899,7 @@ Do_redirect::cfg_init(Config &cfg, CfgStaticData const *)
 Errata
 Do_redirect::invoke(Context &ctx)
 {
-  auto cfg_info = ctx.cfg().named_object<CfgInfo>(KEY);
-  if (!cfg_info) {
-    return {}; // shutdown in progress and config data is already gone.
-  }
-  auto ctx_info = ctx.initialized_storage_for<CtxInfo>(cfg_info->_ctx_span).data();
+  auto ctx_info = ctx.obtain_named_object<CtxInfo>(KEY);
 
   // If the Location view is empty, it hasn't been set and therefore the clean up hook
   // hasn't been set either, so need to do that.
@@ -2976,8 +2948,7 @@ Do_redirect::invoke(Context &ctx)
 Errata
 Do_redirect::fixup(Context &ctx)
 {
-  if ( auto cfg_info = ctx.cfg().named_object<CfgInfo>(KEY) ; cfg_info ) {
-    auto ctx_info = ctx.storage_for(cfg_info->_ctx_span).rebind<CtxInfo>().data();
+  if ( auto ctx_info = ctx.named_object<CtxInfo>(KEY) ; ctx_info ) {
     auto hdr{ctx.proxy_rsp_hdr()};
 
     auto field{hdr.field_obtain(ts::HTTP_FIELD_LOCATION)};