From 2dfa89e24364138776a75ea469c29702f53c0a7a Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Thu, 9 Jan 2025 11:06:37 -0600 Subject: [PATCH] common: fix md_config_cacher_t In its get_tracked_conf_keys() member function, the cacher (in the existing code) initializes a static function-block variable ('keys'), and uses it for registering the observer. But the cacher is instantiated on the type of the configuration value. Thus, multiple cacher objects for which the configuration values are of the same type - share the static 'keys'. Only one of the observers is registered. Note that the code could have been simplified somewhat, if the signature of the get_tracked_conf_keys() function was changed to return 'const char* const *'. Fixes (original PR): https://tracker.ceph.com/issues/69236 Fixes: https://tracker.ceph.com/issues/69530 Signed-off-by: Ronen Friedman (cherry picked from commit 8ac04704574671e72de6f3ba30c1c3baa39a6319) --- src/common/config_cacher.h | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/common/config_cacher.h b/src/common/config_cacher.h index a84bad08eee..f7839c5a679 100644 --- a/src/common/config_cacher.h +++ b/src/common/config_cacher.h @@ -18,21 +18,30 @@ #include "common/config_obs.h" #include "common/config.h" +/** + * A simple class to cache a single configuration value. + * Points to note: + * - as get_tracked_conf_keys() must return a pointer to a null-terminated + * array of C-strings, 'keys' - an array - is used to hold the sole key + * that this observer is interested in. + * - the const cast should be removed once we change the + * get_tracked_conf_keys() to return const char* const * (or something + * similar). + */ template class md_config_cacher_t : public md_config_obs_t { ConfigProxy& conf; - const char* const option_name; + const char* keys[2]; std::atomic value_cache; const char** get_tracked_conf_keys() const override { - const static char* keys[] = { option_name, nullptr }; - return keys; + return const_cast(keys); } void handle_conf_change(const ConfigProxy& conf, const std::set& changed) override { - if (changed.count(option_name)) { - value_cache.store(conf.get_val(option_name)); + if (changed.count(keys[0])) { + value_cache.store(conf.get_val(keys[0])); } } @@ -40,10 +49,10 @@ public: md_config_cacher_t(ConfigProxy& conf, const char* const option_name) : conf(conf), - option_name(option_name) { + keys{option_name, nullptr} { conf.add_observer(this); std::atomic_init(&value_cache, - conf.get_val(option_name)); + conf.get_val(keys[0])); } ~md_config_cacher_t() { -- 2.39.5