From: Ronen Friedman <rfriedma@redhat.com>
Date: Thu, 9 Jan 2025 17:06:37 +0000 (-0600)
Subject: common: fix md_config_cacher_t
X-Git-Tag: testing/wip-vshankar-testing-20250115.164543-debug~9^2~3
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=8ac04704574671e72de6f3ba30c1c3baa39a6319;p=ceph-ci.git

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: https://tracker.ceph.com/issues/69236
Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
---

diff --git a/src/common/config_cacher.h b/src/common/config_cacher.h
index 91b8152dde1..2befb497318 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 <typename ValueT>
 class md_config_cacher_t : public md_config_obs_t {
   ConfigProxy& conf;
-  const char* const option_name;
+  const char* keys[2];
   std::atomic<ValueT> value_cache;
 
   const char** get_tracked_conf_keys() const override {
-    const static char* keys[] = { option_name, nullptr };
-    return keys;
+    return const_cast<const char**>(keys);
   }
 
   void handle_conf_change(const ConfigProxy& conf,
                           const std::set<std::string>& changed) override {
-    if (changed.count(option_name)) {
-      value_cache.store(conf.get_val<ValueT>(option_name));
+    if (changed.count(keys[0])) {
+      value_cache.store(conf.get_val<ValueT>(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<ValueT>(option_name));
+                     conf.get_val<ValueT>(keys[0]));
   }
 
   ~md_config_cacher_t() {