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;