This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 4af1b1cc8b HRW: Better txn slot management (#12566)
4af1b1cc8b is described below

commit 4af1b1cc8b82030440850fcd454c5028a7c248e6
Author: Leif Hedstrom <[email protected]>
AuthorDate: Thu Oct 16 12:49:23 2025 -0600

    HRW: Better txn slot management (#12566)
    
    * HRW: Better txn slot management
    
    This also fixes a real bug, where the txn slot for the state variables could
    overlap / conflict with the control mechanism and its txn slot. They need
    to have different names.
    
    * Add some docs to clarify slots naming
---
 .../api/functions/TSUserArgs.en.rst                |  3 ++-
 plugins/header_rewrite/header_rewrite.cc           |  5 ++---
 plugins/header_rewrite/statement.cc                | 23 +++++++---------------
 plugins/header_rewrite/statement.h                 | 22 ++++++++++-----------
 4 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/doc/developer-guide/api/functions/TSUserArgs.en.rst 
b/doc/developer-guide/api/functions/TSUserArgs.en.rst
index a0b917ba40..aacf9ead1a 100644
--- a/doc/developer-guide/api/functions/TSUserArgs.en.rst
+++ b/doc/developer-guide/api/functions/TSUserArgs.en.rst
@@ -61,7 +61,8 @@ plugin can reserve a slot of a particular type by calling 
:func:`TSUserArgIndexR
    The type for which the plugin intend to reserve a slot. See 
:code:`TSUserArgType` above.
 
 :arg:`name`
-   An identifying name for the plugin that reserved the index. Required.
+   An unique and identifying name for the reserved slot index. A plugin 
registering
+   multiple slots (not recommended!) must make sure the identifier is unique.
 
 :arg:`description`
    An optional description of the use of the arg. This can be :code:`nullptr`.
diff --git a/plugins/header_rewrite/header_rewrite.cc 
b/plugins/header_rewrite/header_rewrite.cc
index 04c3d78abd..89fe252e99 100644
--- a/plugins/header_rewrite/header_rewrite.cc
+++ b/plugins/header_rewrite/header_rewrite.cc
@@ -376,10 +376,9 @@ static void
 setPluginControlValues(TSHttpTxn txnp, RulesConfig *conf)
 {
   if (conf->timezone() != 0 || conf->inboundIpSource() != 0) {
-    ConditionNow temporal_statement; // This could be any statement that use 
the private slot.
-    int          slot = temporal_statement.get_txn_private_slot();
-
+    int             slot = Statement::acquire_txn_private_slot();
     PrivateSlotData private_data;
+
     private_data.raw       = reinterpret_cast<uint64_t>(TSUserArgGet(txnp, 
slot));
     private_data.timezone  = conf->timezone();
     private_data.ip_source = conf->inboundIpSource();
diff --git a/plugins/header_rewrite/statement.cc 
b/plugins/header_rewrite/statement.cc
index 979eb8f146..ca840d7a9b 100644
--- a/plugins/header_rewrite/statement.cc
+++ b/plugins/header_rewrite/statement.cc
@@ -73,14 +73,9 @@ Statement::initialize_hooks()
   add_allowed_hook(TS_HTTP_TXN_CLOSE_HOOK);
 }
 
-void
+int
 Statement::acquire_txn_slot()
 {
-  // Don't do anything if we don't need it
-  if (!need_txn_slot() || _txn_slot >= 0) {
-    return;
-  }
-
   // Only call the index reservation once per plugin load
   static int txn_slot_index = []() -> int {
     int index = -1;
@@ -92,29 +87,25 @@ Statement::acquire_txn_slot()
     return index;
   }();
 
-  _txn_slot = txn_slot_index;
+  return txn_slot_index;
 }
 
-void
+int
 Statement::acquire_txn_private_slot()
 {
-  // Don't do anything if we don't need it
-  if (!need_txn_private_slot() || _txn_private_slot >= 0) {
-    return;
-  }
-
   // Only call the index reservation once per plugin load
   static int txn_private_slot_index = []() -> int {
-    int index = -1;
+    int         index = -1;
+    std::string name  = std::string(PLUGIN_NAME) + "_priv";
 
-    if (TS_ERROR == TSUserArgIndexReserve(TS_USER_ARGS_TXN, PLUGIN_NAME, "HRW 
txn private variables", &index)) {
+    if (TS_ERROR == TSUserArgIndexReserve(TS_USER_ARGS_TXN, name.c_str(), "HRW 
txn private variables", &index)) {
       TSError("[%s] failed to reserve user arg index", PLUGIN_NAME);
       return -1; // Fallback value
     }
     return index;
   }();
 
-  _txn_private_slot = txn_private_slot_index;
+  return txn_private_slot_index;
 }
 
 // Parse NextHop qualifiers
diff --git a/plugins/header_rewrite/statement.h 
b/plugins/header_rewrite/statement.h
index bb7a796687..28e7195128 100644
--- a/plugins/header_rewrite/statement.h
+++ b/plugins/header_rewrite/statement.h
@@ -168,8 +168,13 @@ public:
   {
     TSReleaseAssert(_initialized == false);
     initialize_hooks();
-    acquire_txn_slot();
-    acquire_txn_private_slot();
+
+    if (need_txn_slot()) {
+      _txn_slot = acquire_txn_slot();
+    }
+    if (need_txn_private_slot()) {
+      _txn_private_slot = acquire_txn_private_slot();
+    }
 
     _initialized = true;
   }
@@ -180,19 +185,15 @@ public:
     return _initialized;
   }
 
-  int
-  get_txn_private_slot()
-  {
-    acquire_txn_private_slot();
-    return _txn_private_slot;
-  }
-
   void
   require_resources(const ResourceIDs ids)
   {
     _rsrc = static_cast<ResourceIDs>(_rsrc | ids);
   }
 
+  static int acquire_txn_slot();
+  static int acquire_txn_private_slot();
+
 protected:
   virtual void initialize_hooks();
 
@@ -217,9 +218,6 @@ protected:
   int        _txn_private_slot = -1;
 
 private:
-  void acquire_txn_slot();
-  void acquire_txn_private_slot();
-
   ResourceIDs               _rsrc = RSRC_NONE;
   TSHttpHookID              _hook = TS_HTTP_READ_RESPONSE_HDR_HOOK;
   std::vector<TSHttpHookID> _allowed_hooks;

Reply via email to