From: Kefu Chai <kchai@redhat.com>
Date: Tue, 3 Aug 2021 12:44:01 +0000 (+0800)
Subject: tools/ceph-dencoder: register dencoders in plugin
X-Git-Tag: v16.2.11~358^2~4
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=87feb5734b24ab0177481a7ef9ab7c8666c4ebd7;p=ceph.git

tools/ceph-dencoder: register dencoders in plugin

so we can allocate and deallocate dencoders in the shared library,
instead of allocating them in the shared library, while deallocating
them in the executable.

after this change

- the plugin holds the strong references of the dencoders
- the registry holds the plugins and weak references of dencoders
- the dencoder shared libraries calls the method exposed by plugin
  to alloc/dealloc the dencoders

this change should address the segfault when compiling with Clang.

Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit ae7e79db7b5bc38d8ecde6705932ff2671c9c326)
---

diff --git a/src/tools/ceph-dencoder/ceph_dencoder.cc b/src/tools/ceph-dencoder/ceph_dencoder.cc
index b4974eaea5fd..9ade1a4d7b89 100644
--- a/src/tools/ceph-dencoder/ceph_dencoder.cc
+++ b/src/tools/ceph-dencoder/ceph_dencoder.cc
@@ -63,7 +63,7 @@ void usage(ostream &out)
   out << "  is_deterministic    exit w/ success if type encodes deterministically\n";
 }
 
-vector<DencoderPlugin> load_plugins(DencoderRegistry& registry)
+vector<DencoderPlugin> load_plugins()
 {
   fs::path mod_dir{CEPH_DENC_MOD_DIR};
   if (auto ceph_lib = getenv("CEPH_LIB"); ceph_lib) {
@@ -80,12 +80,6 @@ vector<DencoderPlugin> load_plugins(DencoderRegistry& registry)
     if (!plugin.good()) {
       continue;
     }
-    int n = plugin.register_dencoders(registry);
-    if (n <= 0) {
-      std::cerr << "fail to load dencoders from " << entry << std::endl;
-      continue;
-    }
-    std::cout << "load " << n << " from " << entry << std::endl;
     dencoder_plugins.push_back(std::move(plugin));
   }
   return dencoder_plugins;
@@ -93,8 +87,13 @@ vector<DencoderPlugin> load_plugins(DencoderRegistry& registry)
 
 int main(int argc, const char **argv)
 {
+  vector<DencoderPlugin> plugins = load_plugins();
   DencoderRegistry registry;
-  auto plugins = load_plugins(registry);
+  for (auto& plugin : plugins) {
+    for (auto& [name, denc] : plugin.register_dencoders()) {
+      registry.register_dencoder(name, denc);
+    }
+  }
 
   vector<const char*> args;
   argv_to_vec(argc, argv, args);
@@ -133,7 +132,7 @@ int main(int argc, const char **argv)
 	cerr << "class '" << cname << "' unknown" << std::endl;
 	return 1;
       }
-      den = dencoders[cname].get();
+      den = dencoders[cname];
       den->generate();
     } else if (*i == string("skip")) {
       ++i;
diff --git a/src/tools/ceph-dencoder/common_types.cc b/src/tools/ceph-dencoder/common_types.cc
index fcd8a1eb4332..fa763c3bbd96 100644
--- a/src/tools/ceph-dencoder/common_types.cc
+++ b/src/tools/ceph-dencoder/common_types.cc
@@ -23,9 +23,14 @@ using namespace std;
 #undef TYPE_FEATUREFUL_NOCOPY
 #undef MESSAGE
 
-#include "denc_registry.h"
+#include "denc_plugin.h"
 
-DENC_API void register_dencoders(DencoderRegistry& registry)
+DENC_API void register_dencoders(DencoderPlugin* plugin)
 {
 #include "common_types.h"
 }
+
+DENC_API void unregister_dencoders(DencoderPlugin* plugin)
+{
+  plugin->unregister_dencoders();
+}
diff --git a/src/tools/ceph-dencoder/denc_plugin.h b/src/tools/ceph-dencoder/denc_plugin.h
index 1a0834ec766a..3b17a4f55da8 100644
--- a/src/tools/ceph-dencoder/denc_plugin.h
+++ b/src/tools/ceph-dencoder/denc_plugin.h
@@ -1,11 +1,13 @@
 #include <dlfcn.h>
 #include <filesystem>
+#include <vector>
 
 #include "denc_registry.h"
 
 namespace fs = std::filesystem;
 
 class DencoderPlugin {
+  using dencoders_t = std::vector<std::pair<std::string, Dencoder*>>;
 public:
   DencoderPlugin(const fs::path& path) {
     mod = dlopen(path.c_str(), RTLD_NOW);
@@ -13,6 +15,13 @@ public:
       std::cerr << "failed to dlopen(" << path << "): " << dlerror() << std::endl;
     }
   }
+  DencoderPlugin(DencoderPlugin&& other)
+    : mod{other.mod},
+      dencoders{std::move(other.dencoders)}
+  {
+    other.mod = nullptr;
+    other.dencoders.clear();
+  }
   ~DencoderPlugin() {
 #if !defined(__FreeBSD__)
     if (mod) {
@@ -20,25 +29,50 @@ public:
     }
 #endif
   }
-  int register_dencoders(DencoderRegistry& registry) {
+  const dencoders_t& register_dencoders() {
     static constexpr string_view REGISTER_DENCODERS_FUNCTION = "register_dencoders\0";
 
     assert(mod);
-    using register_dencoders_t = void (*)(DencoderRegistry&);
+    using register_dencoders_t = void (*)(DencoderPlugin*);
     const auto do_register =
       reinterpret_cast<register_dencoders_t>(dlsym(mod, REGISTER_DENCODERS_FUNCTION.data()));
     if (do_register == nullptr) {
-      std::cerr << "failed to dlsym(" << REGISTER_DENCODERS_FUNCTION << ")" << std::endl;
-      return -1;
+      std::cerr << "failed to dlsym(" << REGISTER_DENCODERS_FUNCTION << "): "
+                << dlerror() << std::endl;
+      return dencoders;
     }
-    const unsigned nr_before = registry.get().size();
-    do_register(registry);
-    const unsigned nr_after = registry.get().size();
-    return nr_after - nr_before;
+    do_register(this);
+    return dencoders;
   }
+
   bool good() const {
     return mod != nullptr;
   }
+
+  void unregister_dencoders() {
+    while (!dencoders.empty()) {
+      delete dencoders.back().second;
+      dencoders.pop_back();
+    }
+  }
+  template<typename DencoderT, typename...Args>
+  void emplace(const char* name, Args&&...args) {
+    dencoders.emplace_back(name, new DencoderT(std::forward<Args>(args)...));
+  }
+
 private:
   void *mod = nullptr;
+  dencoders_t dencoders;
 };
+
+#define TYPE(t) plugin->emplace<DencoderImplNoFeature<t>>(#t, false, false);
+#define TYPE_STRAYDATA(t) plugin->emplace<DencoderImplNoFeature<t>>(#t, true, false);
+#define TYPE_NONDETERMINISTIC(t) plugin->emplace<DencoderImplNoFeature<t>>(#t, false, true);
+#define TYPE_FEATUREFUL(t) plugin->emplace<DencoderImplFeatureful<t>>(#t, false, false);
+#define TYPE_FEATUREFUL_STRAYDATA(t) plugin->emplace<DencoderImplFeatureful<t>>(#t, true, false);
+#define TYPE_FEATUREFUL_NONDETERMINISTIC(t) plugin->emplace<DencoderImplFeatureful<t>>(#t, false, true);
+#define TYPE_FEATUREFUL_NOCOPY(t) plugin->emplace<DencoderImplFeaturefulNoCopy<t>>(#t, false, false);
+#define TYPE_NOCOPY(t) plugin->emplace<DencoderImplNoFeatureNoCopy<t>>(#t, false, false);
+#define MESSAGE(t) plugin->emplace<MessageDencoderImpl<t>>(#t);
+
+#define DENC_API extern "C" [[gnu::visibility("default")]]
diff --git a/src/tools/ceph-dencoder/denc_registry.h b/src/tools/ceph-dencoder/denc_registry.h
index 32a016f6b170..dc1db36d3f22 100644
--- a/src/tools/ceph-dencoder/denc_registry.h
+++ b/src/tools/ceph-dencoder/denc_registry.h
@@ -5,6 +5,8 @@
 
 #include <iostream>
 #include <string>
+#include <string_view>
+
 #include "include/buffer_fwd.h"
 #include "msg/Message.h"
 
@@ -225,31 +227,15 @@ public:
 
 class DencoderRegistry
 {
-  using dencoders_t = std::map<std::string, std::unique_ptr<Dencoder>>;
+  using dencoders_t = std::map<std::string_view, Dencoder*>;
 
 public:
   dencoders_t& get() {
     return dencoders;
   }
-
-  template<typename DencoderT, typename...Args>
-  void emplace(const char* name, Args&&...args) {
-    auto dencoder = std::make_unique<DencoderT>(std::forward<Args>(args)...);
-    dencoders.emplace(name, std::move(dencoder));
+  void register_dencoder(std::string_view name, Dencoder* denc) {
+    dencoders.emplace(name, denc);
   }
-
 private:
   dencoders_t dencoders;
 };
-
-#define TYPE(t) registry.emplace<DencoderImplNoFeature<t>>(#t, false, false);
-#define TYPE_STRAYDATA(t) registry.emplace<DencoderImplNoFeature<t>>(#t, true, false);
-#define TYPE_NONDETERMINISTIC(t) registry.emplace<DencoderImplNoFeature<t>>(#t, false, true);
-#define TYPE_FEATUREFUL(t) registry.emplace<DencoderImplFeatureful<t>>(#t, false, false);
-#define TYPE_FEATUREFUL_STRAYDATA(t) registry.emplace<DencoderImplFeatureful<t>>(#t, true, false);
-#define TYPE_FEATUREFUL_NONDETERMINISTIC(t) registry.emplace<DencoderImplFeatureful<t>>(#t, false, true);
-#define TYPE_FEATUREFUL_NOCOPY(t) registry.emplace<DencoderImplFeaturefulNoCopy<t>>(#t, false, false);
-#define TYPE_NOCOPY(t) registry.emplace<DencoderImplNoFeatureNoCopy<t>>(#t, false, false);
-#define MESSAGE(t) registry.emplace<MessageDencoderImpl<t>>(#t);
-
-#define DENC_API extern "C" [[gnu::visibility("default")]]
diff --git a/src/tools/ceph-dencoder/mds_types.cc b/src/tools/ceph-dencoder/mds_types.cc
index 3254bac62d36..94280477a5b5 100644
--- a/src/tools/ceph-dencoder/mds_types.cc
+++ b/src/tools/ceph-dencoder/mds_types.cc
@@ -23,9 +23,14 @@ using namespace std;
 #undef TYPE_FEATUREFUL_NOCOPY
 #undef MESSAGE
 
-#include "denc_registry.h"
+#include "denc_plugin.h"
 
-DENC_API void register_dencoders(DencoderRegistry& registry)
+DENC_API void register_dencoders(DencoderPlugin* plugin)
 {
 #include "mds_types.h"
 }
+
+DENC_API void unregister_dencoders(DencoderPlugin* plugin)
+{
+  plugin->unregister_dencoders();
+}
diff --git a/src/tools/ceph-dencoder/osd_types.cc b/src/tools/ceph-dencoder/osd_types.cc
index f7dc1e8d89dd..13a90685bc85 100644
--- a/src/tools/ceph-dencoder/osd_types.cc
+++ b/src/tools/ceph-dencoder/osd_types.cc
@@ -23,12 +23,17 @@ using namespace std;
 #undef TYPE_FEATUREFUL_NOCOPY
 #undef MESSAGE
 
-#include "denc_registry.h"
+#include "denc_plugin.h"
 
 // cannot initialize dencoders when initializing static variables, as some of
 // the types are allocated using mempool, and the mempools are initialized as
 // static variables.
-DENC_API void register_dencoders(DencoderRegistry& registry)
+DENC_API void register_dencoders(DencoderPlugin* plugin)
 {
 #include "osd_types.h"
 }
+
+DENC_API void unregister_dencoders(DencoderPlugin* plugin)
+{
+  plugin->unregister_dencoders();
+}
diff --git a/src/tools/ceph-dencoder/rbd_types.cc b/src/tools/ceph-dencoder/rbd_types.cc
index 361a2a75e295..e04efc30d528 100644
--- a/src/tools/ceph-dencoder/rbd_types.cc
+++ b/src/tools/ceph-dencoder/rbd_types.cc
@@ -23,9 +23,14 @@ using namespace std;
 #undef TYPE_FEATUREFUL_NOCOPY
 #undef MESSAGE
 
-#include "denc_registry.h"
+#include "denc_plugin.h"
 
-DENC_API void register_dencoders(DencoderRegistry& registry)
+DENC_API void register_dencoders(DencoderPlugin* plugin)
 {
 #include "rbd_types.h"
 }
+
+DENC_API void unregister_dencoders(DencoderPlugin* plugin)
+{
+  plugin->unregister_dencoders();
+}
diff --git a/src/tools/ceph-dencoder/rgw_types.cc b/src/tools/ceph-dencoder/rgw_types.cc
index 1b8adece4e1d..79688b5343f5 100644
--- a/src/tools/ceph-dencoder/rgw_types.cc
+++ b/src/tools/ceph-dencoder/rgw_types.cc
@@ -23,9 +23,14 @@ using namespace std;
 #undef TYPE_FEATUREFUL_NOCOPY
 #undef MESSAGE
 
-#include "denc_registry.h"
+#include "denc_plugin.h"
 
-DENC_API void register_dencoders(DencoderRegistry& registry)
+DENC_API void register_dencoders(DencoderPlugin* plugin)
 {
 #include "rgw_types.h"
 }
+
+DENC_API void unregister_dencoders(DencoderPlugin* plugin)
+{
+  plugin->unregister_dencoders();
+}