diff --git a/include/ts/ts.h b/include/ts/ts.h index 9b62d64fe8c..a386ea9ec05 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 TSMutexLockGuard +{ +public: + [[nodiscard]] 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 90ce73345ed..09979e1a58c 100644 --- a/plugins/background_fetch/background_fetch.cc +++ b/plugins/background_fetch/background_fetch.cc @@ -117,14 +117,13 @@ class BgFetchState { 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 6dd3ec82259..2274e1f141c 100644 --- a/plugins/certifier/certifier.cc +++ b/plugins/certifier/certifier.cc @@ -132,12 +132,12 @@ class SslLRUList 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 @@ class SslLRUList ssl_data->scheduled = true; } } - TSMutexUnlock(list_mutex); return ref_ctx; } @@ -265,27 +264,24 @@ class SslLRUList 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 @@ class SslLRUList 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 011d5e1d37d..a2744f7ab2f 100644 --- a/plugins/experimental/cache_fill/background_fetch.h +++ b/plugins/experimental/cache_fill/background_fetch.h @@ -99,14 +99,13 @@ class BgFetchState { 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 520b12d2745..1853861c496 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 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 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 02a2bbff8c1..0f1b4f2f46f 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 1e98cfd3fd7..1bb42e85e28 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 ec96d5c9c2f..c7c2b636f36 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 6e7c32a31ae..e2c16170faf 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(TSContDataGet(contp)); - auto *old_wasm = static_cast(c->wasm()); - TSMutexLock(old_wasm->mutex()); + auto *old_wasm = static_cast(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 02b07817118..eeaa4f03a5b 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 b14b918eabc..45a80557c55 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(1)); } - TSMutexUnlock(lmutex); return 0; } diff --git a/plugins/lua/ts_lua_util.cc b/plugins/lua/ts_lua_util.cc index bf1577f68f9..10baf236b79 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 f35b137aa3c..b0d5c7615c1 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 2a1b7082a19..66ae24d561a 100644 --- a/plugins/prefetch/fetch.h +++ b/plugins/prefetch/fetch.h @@ -140,7 +140,7 @@ class BgFetchStates BgFetchState *state; std::map::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 @@ class BgFetchStates state = new BgFetchState(); _prefetchStates->_states[space] = state; } - TSMutexUnlock(_prefetchStates->_lock); return state; }