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>

diff --git a/src/tools/ceph-dencoder/ceph_dencoder.cc b/src/tools/ceph-dencoder/ceph_dencoder.cc
index 30e7eef945a7d19ec1ea942f7decb64625994f29..03f0adf99f5dc21027fd77cc51594954b62e5bfc 100644 (file)
--- a/src/tools/ceph-dencoder/ceph_dencoder.cc
+++ b/src/tools/ceph-dencoder/ceph_dencoder.cc
@@ -60,7 +60,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) {
@@ -83,12 +83,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;
@@ -96,8 +90,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);
@@ -136,7 +135,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 fcd8a1eb4332d7068b7cf63897b6765250a525dc..fa763c3bbd96b213c75b9ea4f21b796fa4850a04 100644 (file)
--- 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 1a0834ec766a7c0bbbdcdfac2cd984c2795e4200..3b17a4f55da8e31c4bb6169f3a5a9d9044290b8d 100644 (file)
--- 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 32a016f6b1702a1422453af0c69a9ea98bf8d3bc..dc1db36d3f221baf6f1ca89957e8a95a8a876b7f 100644 (file)
--- 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 3254bac62d368eb36d4dcf9e89cdb2bc6f02c088..94280477a5b56bc5542f806edf65d797c7e9f3ea 100644 (file)
--- 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 f7dc1e8d89ddb43b5ec2125094e1b5ff87082a1b..13a90685bc8545d08f6345bff0c858105dbd1ffd 100644 (file)
--- 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 361a2a75e295bb9fba06a7141055fb8a21834bcd..e04efc30d52877e9996c768a347a645a87e7868d 100644 (file)
--- 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 1b8adece4e1d4782b72a9ea7db64644422ea88b6..79688b5343f5737a4852062f7fe87dd220e2cb3b 100644 (file)
--- 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();
+}