This is an automated email from the ASF dual-hosted git repository.
bneradt 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 094c755d7d Add TSMutex lock guard (#13188)
094c755d7d is described below
commit 094c755d7d2c503187e569a5df0d047453573125
Author: Brian Neradt <[email protected]>
AuthorDate: Tue Jun 23 12:57:11 2026 -0500
Add TSMutex lock guard (#13188)
Plugin code that protects small critical sections with TSMutex has to pair
every early return with a matching unlock. That pattern is easy to get wrong
and makes the intended lock lifetime harder to see.
This adds a small TSMutexLockGuard helper to the plugin API and uses it in
plugin code where the mutex naturally stays locked until a return path.
---
include/ts/ts.h | 20 +++++++++++++
plugins/background_fetch/background_fetch.cc | 3 +-
plugins/certifier/certifier.cc | 35 ++++++++++------------
plugins/experimental/cache_fill/background_fetch.h | 3 +-
plugins/experimental/rate_limit/ip_reputation.cc | 16 +++-------
.../experimental/stale_response/stale_response.cc | 7 +++--
plugins/experimental/system_stats/system_stats.cc | 4 +--
plugins/experimental/wasm/ats_context.cc | 7 ++---
plugins/experimental/wasm/wasm_main.cc | 7 ++---
plugins/lua/ts_lua.cc | 5 +---
plugins/lua/ts_lua_fetch.cc | 3 +-
plugins/lua/ts_lua_util.cc | 9 +-----
plugins/prefetch/fetch.cc | 4 +--
plugins/prefetch/fetch.h | 3 +-
14 files changed, 56 insertions(+), 70 deletions(-)
diff --git a/include/ts/ts.h b/include/ts/ts.h
index ad266054cb..5631bee0f6 100644
--- a/include/ts/ts.h
+++ b/include/ts/ts.h
@@ -1168,6 +1168,26 @@ TSReturnCode TSMutexLockTry(TSMutex mutexp);
void TSMutexUnlock(TSMutex mutexp);
+/** Scoped lock guard for a @c TSMutex.
+
+ Locks @a mutexp on construction and unlocks it when the guard leaves scope.
+ */
+class [[nodiscard]] TSMutexLockGuard
+{
+public:
+ explicit TSMutexLockGuard(TSMutex mutexp) : m_mutex(mutexp) {
TSMutexLock(m_mutex); }
+
+ TSMutexLockGuard(const TSMutexLockGuard &) = delete;
+ TSMutexLockGuard &operator=(const TSMutexLockGuard &) = delete;
+ TSMutexLockGuard(TSMutexLockGuard &&) = delete;
+ TSMutexLockGuard &operator=(TSMutexLockGuard &&) = delete;
+
+ ~TSMutexLockGuard() { TSMutexUnlock(m_mutex); }
+
+private:
+ TSMutex m_mutex = nullptr;
+};
+
/* --------------------------------------------------------------------------
cachekey */
/**
diff --git a/plugins/background_fetch/background_fetch.cc
b/plugins/background_fetch/background_fetch.cc
index 90ce73345e..09979e1a58 100644
--- a/plugins/background_fetch/background_fetch.cc
+++ b/plugins/background_fetch/background_fetch.cc
@@ -117,14 +117,13 @@ public:
{
bool ret;
- TSMutexLock(_lock);
+ TSMutexLockGuard lock(_lock);
if (_urls.end() == _urls.find(url)) {
ret = false;
} else {
_urls.erase(url);
ret = true;
}
- TSMutexUnlock(_lock);
return ret;
}
diff --git a/plugins/certifier/certifier.cc b/plugins/certifier/certifier.cc
index 6dd3ec8225..2274e1f141 100644
--- a/plugins/certifier/certifier.cc
+++ b/plugins/certifier/certifier.cc
@@ -132,12 +132,12 @@ public:
SSL_CTX *
lookup_and_create(const char *servername, void *edata, bool &wontdo)
{
- SslData *ssl_data = nullptr;
- scoped_SslData scoped_ssl_data = nullptr;
- SSL_CTX *ref_ctx = nullptr;
- std::string commonName(servername);
- TSMutexLock(list_mutex);
- auto dataItr = cnDataMap.find(commonName);
+ SslData *ssl_data = nullptr;
+ scoped_SslData scoped_ssl_data = nullptr;
+ SSL_CTX *ref_ctx = nullptr;
+ std::string commonName(servername);
+ TSMutexLockGuard lock(list_mutex);
+ auto dataItr = cnDataMap.find(commonName);
/// If such a context exists in dict
if (dataItr != cnDataMap.end()) {
/// Reuse context if already built, self queued if not
@@ -165,7 +165,6 @@ public:
ssl_data->scheduled = true;
}
}
- TSMutexUnlock(list_mutex);
return ref_ctx;
}
@@ -265,27 +264,24 @@ public:
SslData *
get_newest()
{
- TSMutexLock(list_mutex);
- SslData *ret = head;
- TSMutexUnlock(list_mutex);
+ TSMutexLockGuard lock(list_mutex);
+ SslData *ret = head;
return ret;
}
SslData *
get_oldest()
{
- TSMutexLock(list_mutex);
- SslData *ret = tail;
- TSMutexUnlock(list_mutex);
+ TSMutexLockGuard lock(list_mutex);
+ SslData *ret = tail;
return ret;
}
int
get_size()
{
- TSMutexLock(list_mutex);
- int ret = size;
- TSMutexUnlock(list_mutex);
+ TSMutexLockGuard lock(list_mutex);
+ int ret = size;
return ret;
}
@@ -293,14 +289,13 @@ public:
int
set_schedule(const std::string &commonName, bool flag)
{
- int ret = -1;
- TSMutexLock(list_mutex);
- auto iter = cnDataMap.find(commonName);
+ int ret = -1;
+ TSMutexLockGuard lock(list_mutex);
+ auto iter = cnDataMap.find(commonName);
if (iter != cnDataMap.end()) {
iter->second->scheduled = flag;
ret = 0;
}
- TSMutexUnlock(list_mutex);
return ret;
}
};
diff --git a/plugins/experimental/cache_fill/background_fetch.h
b/plugins/experimental/cache_fill/background_fetch.h
index 011d5e1d37..a2744f7ab2 100644
--- a/plugins/experimental/cache_fill/background_fetch.h
+++ b/plugins/experimental/cache_fill/background_fetch.h
@@ -99,14 +99,13 @@ public:
{
bool ret;
- TSMutexLock(_lock);
+ TSMutexLockGuard lock(_lock);
if (_urls.end() == _urls.find(url)) {
ret = false;
} else {
_urls.erase(url);
ret = true;
}
- TSMutexUnlock(_lock);
return ret;
}
diff --git a/plugins/experimental/rate_limit/ip_reputation.cc
b/plugins/experimental/rate_limit/ip_reputation.cc
index 520b12d274..1853861c49 100644
--- a/plugins/experimental/rate_limit/ip_reputation.cc
+++ b/plugins/experimental/rate_limit/ip_reputation.cc
@@ -144,7 +144,7 @@ SieveLru::parseYaml(const YAML::Node &node)
std::tuple<uint32_t, uint32_t>
SieveLru::increment(KeyClass key)
{
- TSMutexLock(_lock);
+ TSMutexLockGuard lock(_lock);
TSAssert(_initialized);
auto map_it = _map.find(key);
@@ -166,7 +166,6 @@ SieveLru::increment(KeyClass key)
lru->push_front({key, 1, entryBucket(), SystemClock::now()});
}
_map[key] = lru->begin();
- TSMutexUnlock(_lock);
return {entryBucket(), 1};
} else {
@@ -213,7 +212,6 @@ SieveLru::increment(KeyClass key)
lru->moveTop(lru.get(), map_item);
}
}
- TSMutexUnlock(_lock);
return {bucket, count};
}
@@ -223,21 +221,17 @@ SieveLru::increment(KeyClass key)
std::tuple<uint32_t, uint32_t>
SieveLru::lookup(KeyClass key) const
{
- TSMutexLock(_lock);
+ TSMutexLockGuard lock(_lock);
TSAssert(_initialized);
auto map_it = _map.find(key);
if (_map.end() == map_it) {
- TSMutexUnlock(_lock);
-
return {0, entryBucket()}; // Nothing found, return 0 hits and the entry
bucket #
} else {
auto &[map_key, map_item] = *map_it;
auto &[list_key, count, bucket, added] = *map_item;
- TSMutexUnlock(_lock);
-
return {bucket, count};
}
}
@@ -247,7 +241,7 @@ SieveLru::lookup(KeyClass key) const
int32_t
SieveLru::move_bucket(KeyClass key, uint32_t to_bucket)
{
- TSMutexLock(_lock);
+ TSMutexLockGuard lock(_lock);
TSAssert(_initialized);
auto map_it = _map.find(key);
@@ -289,7 +283,6 @@ SieveLru::move_bucket(KeyClass key, uint32_t to_bucket)
added = SystemClock::now();
}
}
- TSMutexUnlock(_lock);
return to_bucket; // Just as a convenience, return the destination bucket
for this entry
}
@@ -336,7 +329,7 @@ SieveBucket::memorySize() const
size_t
SieveLru::memoryUsed() const
{
- TSMutexLock(_lock);
+ TSMutexLockGuard lock(_lock);
TSAssert(_initialized);
size_t total = sizeof(SieveLru);
@@ -347,7 +340,6 @@ SieveLru::memoryUsed() const
total += _map.size() * (sizeof(void *) + sizeof(SieveBucket::iterator));
total += _map.bucket_count() * (sizeof(size_t) + sizeof(void *));
- TSMutexUnlock(_lock);
return total;
}
diff --git a/plugins/experimental/stale_response/stale_response.cc
b/plugins/experimental/stale_response/stale_response.cc
index 02a2bbff8c..0f1b4f2f46 100644
--- a/plugins/experimental/stale_response/stale_response.cc
+++ b/plugins/experimental/stale_response/stale_response.cc
@@ -202,11 +202,12 @@ free_state_info(StateInfo *state)
int64_t
aync_memory_total_add(ConfigInfo *plugin_config, int64_t change)
{
- int64_t total;
- TSMutexLock(plugin_config->body_data_mutex);
+ int64_t total;
+ TSMutexLockGuard lock(plugin_config->body_data_mutex);
+
plugin_config->body_data_memory_usage += change;
total =
plugin_config->body_data_memory_usage;
- TSMutexUnlock(plugin_config->body_data_mutex);
+
return total;
}
/*-----------------------------------------------------------------------------------------------*/
diff --git a/plugins/experimental/system_stats/system_stats.cc
b/plugins/experimental/system_stats/system_stats.cc
index 1e98cfd3fd..1bb42e85e2 100644
--- a/plugins/experimental/system_stats/system_stats.cc
+++ b/plugins/experimental/system_stats/system_stats.cc
@@ -103,7 +103,7 @@ statAdd(const char *name, TSRecordDataType record_type,
TSMutex create_mutex)
{
int stat_id = -1;
- TSMutexLock(create_mutex);
+ TSMutexLockGuard lock(create_mutex);
if (TS_ERROR == TSStatFindName(name, &stat_id)) {
stat_id = TSStatCreate(name, record_type, TS_STAT_NON_PERSISTENT,
TS_STAT_SYNC_SUM);
@@ -114,8 +114,6 @@ statAdd(const char *name, TSRecordDataType record_type,
TSMutex create_mutex)
}
}
- TSMutexUnlock(create_mutex);
-
return stat_id;
}
diff --git a/plugins/experimental/wasm/ats_context.cc
b/plugins/experimental/wasm/ats_context.cc
index ec96d5c9c2..c7c2b636f3 100644
--- a/plugins/experimental/wasm/ats_context.cc
+++ b/plugins/experimental/wasm/ats_context.cc
@@ -496,9 +496,9 @@ Context::getConfiguration()
WasmResult
Context::setTimerPeriod(std::chrono::milliseconds period, uint32_t
*timer_token_ptr)
{
- Wasm *wasm = this->wasm();
- Context *root_context = this->root_context();
- TSMutexLock(wasm->mutex());
+ Wasm *wasm = this->wasm();
+ Context *root_context = this->root_context();
+ TSMutexLockGuard lock(wasm->mutex());
if (!wasm->existsTimerPeriod(root_context->id())) {
Dbg(dbg_ctl, "[%s] no previous timer period set", __FUNCTION__);
TSCont contp = root_context->scheduler_cont();
@@ -511,7 +511,6 @@ Context::setTimerPeriod(std::chrono::milliseconds period,
uint32_t *timer_token_
wasm->setTimerPeriod(root_context->id(), period);
*timer_token_ptr = 0;
- TSMutexUnlock(wasm->mutex());
return WasmResult::Ok;
}
diff --git a/plugins/experimental/wasm/wasm_main.cc
b/plugins/experimental/wasm/wasm_main.cc
index 6e7c32a31a..e2c16170fa 100644
--- a/plugins/experimental/wasm/wasm_main.cc
+++ b/plugins/experimental/wasm/wasm_main.cc
@@ -304,14 +304,13 @@ schedule_handler(TSCont contp, TSEvent /*event*/, void *
/*data*/)
auto *c = static_cast<ats_wasm::Context *>(TSContDataGet(contp));
- auto *old_wasm = static_cast<ats_wasm::Wasm *>(c->wasm());
- TSMutexLock(old_wasm->mutex());
+ auto *old_wasm = static_cast<ats_wasm::Wasm *>(c->wasm());
+ TSMutexLockGuard lock(old_wasm->mutex());
c->onTick(0); // use 0 as token
if (wasm_config->configs.empty()) {
TSError("[wasm][%s] Configuration objects are empty", __FUNCTION__);
- TSMutexUnlock(old_wasm->mutex());
return 0;
}
@@ -365,8 +364,6 @@ schedule_handler(TSCont contp, TSEvent /*event*/, void *
/*data*/)
Dbg(ats_wasm::dbg_ctl, "[%s] config wasm has changed. thus not
scheduling", __FUNCTION__);
}
- TSMutexUnlock(old_wasm->mutex());
-
return 0;
}
diff --git a/plugins/lua/ts_lua.cc b/plugins/lua/ts_lua.cc
index 02b0781711..eeaa4f03a5 100644
--- a/plugins/lua/ts_lua.cc
+++ b/plugins/lua/ts_lua.cc
@@ -525,7 +525,7 @@ ts_lua_remap_plugin_init(void *ih, TSHttpTxn rh,
TSRemapRequestInfo *rri)
pthread_setspecific(lua_state_key, main_ctx);
}
- TSMutexLock(main_ctx->mutexp);
+ TSMutexLockGuard lock(main_ctx->mutexp);
http_ctx = ts_lua_create_http_ctx(main_ctx, instance_conf);
@@ -551,7 +551,6 @@ ts_lua_remap_plugin_init(void *ih, TSHttpTxn rh,
TSRemapRequestInfo *rri)
if (lua_type(L, -1) != LUA_TFUNCTION) {
lua_pop(L, 1);
ts_lua_destroy_http_ctx(http_ctx);
- TSMutexUnlock(main_ctx->mutexp);
return TSREMAP_NO_REMAP;
}
@@ -574,8 +573,6 @@ ts_lua_remap_plugin_init(void *ih, TSHttpTxn rh,
TSRemapRequestInfo *rri)
ts_lua_destroy_http_ctx(http_ctx);
}
- TSMutexUnlock(main_ctx->mutexp);
-
return TSRemapStatus(ret);
}
diff --git a/plugins/lua/ts_lua_fetch.cc b/plugins/lua/ts_lua_fetch.cc
index b14b918eab..45a80557c5 100644
--- a/plugins/lua/ts_lua_fetch.cc
+++ b/plugins/lua/ts_lua_fetch.cc
@@ -542,7 +542,7 @@ ts_lua_fetch_multi_handler(TSCont contp, TSEvent event
ATS_UNUSED, void *edata)
}
// all finish
- TSMutexLock(lmutex);
+ TSMutexLockGuard lock(lmutex);
if (fmi->total == 1 && !fmi->multi) {
ts_lua_fill_one_result(L, fi);
@@ -559,7 +559,6 @@ ts_lua_fetch_multi_handler(TSCont contp, TSEvent event
ATS_UNUSED, void *edata)
TSContCall(ci->contp, TSEvent(TS_LUA_EVENT_COROUTINE_CONT),
reinterpret_cast<void *>(1));
}
- TSMutexUnlock(lmutex);
return 0;
}
diff --git a/plugins/lua/ts_lua_util.cc b/plugins/lua/ts_lua_util.cc
index bf1577f68f..10baf236b7 100644
--- a/plugins/lua/ts_lua_util.cc
+++ b/plugins/lua/ts_lua_util.cc
@@ -289,7 +289,7 @@ ts_lua_add_module(ts_lua_instance_conf *conf,
ts_lua_main_ctx *arr, int n, int a
conf->_first = (i == 0) ? 1 : 0;
conf->_last = (i == n - 1) ? 1 : 0;
- TSMutexLock(arr[i].mutexp);
+ TSMutexLockGuard lock(arr[i].mutexp);
L = arr[i].lua;
@@ -308,7 +308,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf,
ts_lua_main_ctx *arr, int n, int a
if (luaL_loadstring(L, conf->content)) {
snprintf(errbuf, errbuf_size, "[%s] luaL_loadstring failed: %s",
__FUNCTION__, lua_tostring(L, -1));
lua_pop(L, 1);
- TSMutexUnlock(arr[i].mutexp);
return -1;
}
@@ -316,7 +315,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf,
ts_lua_main_ctx *arr, int n, int a
if (luaL_loadfile(L, conf->script)) {
snprintf(errbuf, errbuf_size, "[%s] luaL_loadfile %s failed: %s",
__FUNCTION__, conf->script, lua_tostring(L, -1));
lua_pop(L, 1);
- TSMutexUnlock(arr[i].mutexp);
return -1;
}
}
@@ -324,7 +322,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf,
ts_lua_main_ctx *arr, int n, int a
if (lua_pcall(L, 0, 0, 0)) {
snprintf(errbuf, errbuf_size, "[%s] lua_pcall %s failed: %s",
__FUNCTION__, conf->script, lua_tostring(L, -1));
lua_pop(L, 1);
- TSMutexUnlock(arr[i].mutexp);
return -1;
}
@@ -346,7 +343,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf,
ts_lua_main_ctx *arr, int n, int a
if (lua_pcall(L, 1, 1, 0)) {
snprintf(errbuf, errbuf_size, "[%s] lua_pcall %s failed: %s",
__FUNCTION__, conf->script, lua_tostring(L, -1));
lua_pop(L, 1);
- TSMutexUnlock(arr[i].mutexp);
return -1;
}
@@ -354,7 +350,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf,
ts_lua_main_ctx *arr, int n, int a
lua_pop(L, 1);
if (ret) {
- TSMutexUnlock(arr[i].mutexp);
return -1; /* script parse error */
}
@@ -375,8 +370,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf,
ts_lua_main_ctx *arr, int n, int a
} else {
Dbg(dbg_ctl, "ljgc = %d, NOT running LuaJIT Garbage Collector...",
conf->ljgc);
}
-
- TSMutexUnlock(arr[i].mutexp);
}
return 0;
diff --git a/plugins/prefetch/fetch.cc b/plugins/prefetch/fetch.cc
index f35b137aa3..b0d5c7615c 100644
--- a/plugins/prefetch/fetch.cc
+++ b/plugins/prefetch/fetch.cc
@@ -231,7 +231,7 @@ BgFetchState::init(const PrefetchConfig &config)
TSMutexUnlock(_lock);
/* Initialize fetching policy */
- TSMutexLock(_policyLock);
+ TSMutexLockGuard policy_lock(_policyLock);
if (!config.getFetchPolicy().empty() && 0 !=
config.getFetchPolicy().compare("simple")) {
status &= initializePolicy(_policy, config.getFetchPolicy().c_str());
@@ -242,8 +242,6 @@ BgFetchState::init(const PrefetchConfig &config)
PrefetchDebug("Policy not specified or 'simple' policy chosen (skipping)");
}
- TSMutexUnlock(_policyLock);
-
return status;
}
diff --git a/plugins/prefetch/fetch.h b/plugins/prefetch/fetch.h
index 2a1b7082a1..66ae24d561 100644
--- a/plugins/prefetch/fetch.h
+++ b/plugins/prefetch/fetch.h
@@ -140,7 +140,7 @@ public:
BgFetchState *state;
std::map<String, BgFetchState *>::iterator it;
- TSMutexLock(_prefetchStates->_lock);
+ TSMutexLockGuard lock(_prefetchStates->_lock);
it = _prefetchStates->_states.find(space);
if (it != _prefetchStates->_states.end()) {
state = it->second;
@@ -148,7 +148,6 @@ public:
state = new BgFetchState();
_prefetchStates->_states[space] = state;
}
- TSMutexUnlock(_prefetchStates->_lock);
return state;
}
