Catalog and loader cleanup

phlex/app/load_module.cpp

@@ -6,11 +6,11 @@
 #include "phlex/source.hpp"
 
 #include "boost/algorithm/string.hpp"
-#include "boost/dll/import.hpp"
+#include "boost/dll/shared_library.hpp"
 #include "boost/json.hpp"
 
 #include <cstdlib>
-#include <functional>
+#include <optional>
 #include <string>
 #include <string_view>
 
@@ -21,27 +21,58 @@ namespace phlex::detail {
   namespace {
     constexpr std::string_view pymodule_name{"pymodule"};
 
-    // If factory function goes out of scope, then the library is unloaded...and that's
-    // bad.
+    // The shared_library member in each wrapper struct keeps the loaded .so
+    // alive for the lifetime of the wrapper.  If it goes out of scope, the
+    // library is unloaded and the stored function pointer becomes invalid.
+    struct module_plugin {
+      boost::dll::shared_library lib;
+      internal::module_creator_t* fn{};
+
+      void operator()(module_graph_proxy<void_tag> proxy, configuration const& config) const
+      {
+        fn(std::move(proxy), config);
+      }
+    };
+
+    struct source_plugin {
+      boost::dll::shared_library lib;
+      internal::source_creator_t* fn{};
+
+      void operator()(source_bundle bundle, configuration const& config) const
+      {
+        fn(bundle, config);
+      }
+    };
+
+    struct driver_plugin {
+      boost::dll::shared_library lib;
+      internal::driver_shim_t* fn{};
+
+      void operator()(driver_proxy proxy, configuration const& config, driver_bundle* out) const
+      {
+        fn(proxy, config, out);
+      }
+    };
+
     // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-    std::vector<std::function<internal::module_creator_t>> create_module;
+    std::vector<module_plugin> create_module;
     // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-    std::vector<std::function<internal::source_creator_t>> create_source;
+    std::vector<source_plugin> create_source;
     // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
-    std::function<internal::driver_shim_t> create_driver;
+    std::optional<driver_plugin> create_driver;
 
     template <typename creator_t>
-    std::function<creator_t> plugin_loader(std::string const& spec, std::string const& symbol_name)
+    std::pair<boost::dll::shared_library, creator_t*> plugin_loader(std::string const& spec,
+                                                                    std::string const& symbol_name)
     {
       // Called during single-threaded graph construction
       char const* plugin_path_ptr =
         std::getenv("PHLEX_PLUGIN_PATH"); // NOLINT(concurrency-mt-unsafe)
       if (!plugin_path_ptr)
         throw std::runtime_error("PHLEX_PLUGIN_PATH has not been set.");
 
-      using namespace boost;
       std::vector<std::string> subdirs;
-      split(subdirs, plugin_path_ptr, is_any_of(":"));
+      boost::split(subdirs, plugin_path_ptr, boost::is_any_of(":"));
 
       // FIXME: Need to test to ensure that first match wins.
       for (auto const& subdir : subdirs) {
@@ -50,9 +81,11 @@ namespace phlex::detail {
         if (exists(shared_library_path)) {
           // Load pymodule with rtld_global to make Python symbols available to extension modules
           // (e.g., NumPy). Load all other plugins with rtld_local (default) to avoid symbol collisions.
-          auto const load_mode =
-            (spec == pymodule_name) ? dll::load_mode::rtld_global : dll::load_mode::default_mode;
-          return dll::import_symbol<creator_t>(shared_library_path, symbol_name, load_mode);
+          auto const load_mode = (spec == pymodule_name) ? boost::dll::load_mode::rtld_global
+                                                         : boost::dll::load_mode::default_mode;
+          boost::dll::shared_library lib{shared_library_path, load_mode};
+          auto* fn = &lib.get<creator_t>(symbol_name);
+          return {std::move(lib), fn};
         }
       }
       throw std::runtime_error("Could not locate library with specification '"s + spec +
@@ -89,8 +122,8 @@ namespace phlex::detail {
     auto const adjusted_config = internal::adjust_config(label, std::move(raw_config));
 
     auto const& spec = value_to<std::string>(adjusted_config.at("cpp"));
-    auto& creator =
-      create_module.emplace_back(plugin_loader<internal::module_creator_t>(spec, "create_module"));
+    auto [lib, fn] = plugin_loader<internal::module_creator_t>(spec, "create_module");
+    auto& creator = create_module.emplace_back(module_plugin{std::move(lib), fn});
 
     configuration const config{adjusted_config};
     creator(g.module_proxy(config), config);
@@ -101,8 +134,8 @@ namespace phlex::detail {
     auto const adjusted_config = internal::adjust_config(label, std::move(raw_config));
 
     auto const& spec = value_to<std::string>(adjusted_config.at("cpp"));
-    auto& creator =
-      create_source.emplace_back(plugin_loader<internal::source_creator_t>(spec, "create_source"));
+    auto [lib, fn] = plugin_loader<internal::source_creator_t>(spec, "create_source");
+    auto& creator = create_source.emplace_back(source_plugin{std::move(lib), fn});
 
     // FIXME: Should probably use the parameter name (e.g.) 'plugin_label' instead of
     //        'module_label', but that requires adjusting other parts of the system
@@ -121,9 +154,10 @@ namespace phlex::detail {
     // False positive: clang-analyzer cannot trace ownership through Boost's is_any_of<char>
     // internal reference counting in classification.hpp.
     // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks,clang-analyzer-cplusplus.NewDelete)
-    create_driver = plugin_loader<internal::driver_shim_t>(spec, "create_driver");
+    auto [lib, fn] = plugin_loader<internal::driver_shim_t>(spec, "create_driver");
+    create_driver.emplace(driver_plugin{std::move(lib), fn});
     driver_bundle result;
-    create_driver(g.driver_proxy(required_sources), config, &result);
+    (*create_driver)(g.driver_proxy(required_sources), config, &result);
     g.add_driver(result);
   }
 }

phlex/core/glue.hpp

@@ -35,7 +35,7 @@ namespace phlex::detail {
  *
  * The glue class acts as a registration helper that allows binding user-defined functions and algorithms
  * to nodes in a TBB flow graph. It provides methods to create different types of processing nodes like
- * fold, transform, observe, predicate etc.
+ * fold, observe, output, predicate, transform, and unfold.
  *
  * @tparam T The type of the object that contains the user-defined functions/algorithms to be registered.
  *           This object is stored as a shared pointer and its methods are bound to the created nodes.

phlex/core/graph_proxy.hpp

@@ -39,10 +39,7 @@ namespace phlex::detail {
                 tbb::flow::graph& g,
                 node_catalog& nodes,
                 std::vector<std::string>& errors)
-      requires(not is_bound_object<T>)
-      : config_{&config}, graph_{g}, nodes_{nodes}, errors_{errors}
-    {
-    }
+      requires(not is_bound_object<T>);
 
     /// @brief Binds a user algorithm object of type @p U to this proxy.
     ///
@@ -51,103 +48,58 @@ namespace phlex::detail {
     /// be registered as algorithm nodes.
     template <typename U, typename... Args>
     graph_proxy<U> make(Args&&... args)
-      requires(not is_bound_object<T>)
-    {
-      return bind_to<graph_proxy, U>(std::forward<Args>(args)...);
-    }
+      requires(not is_bound_object<T>);
 
     /// @brief Registers a fold algorithm node.
     template <typename... InitArgs>
     auto fold(std::string name,
               is_fold_like auto f,
               concurrency c = concurrency::serial,
               std::string partition = "job",
-              InitArgs&&... init_args)
-    {
-      return create_glue().fold(std::move(name),
-                                std::move(f),
-                                c,
-                                std::move(partition),
-                                std::forward<InitArgs>(init_args)...);
-    }
+              InitArgs&&... init_args);
 
     /// @brief Registers an observer node.
-    auto observe(std::string name, is_observer_like auto f, concurrency c = concurrency::serial)
-    {
-      return create_glue().observe(std::move(name), std::move(f), c);
-    }
+    auto observe(std::string name, is_observer_like auto f, concurrency c = concurrency::serial);
 
     /// @brief Registers a predicate node.
-    auto predicate(std::string name, is_predicate_like auto f, concurrency c = concurrency::serial)
-    {
-      return create_glue().predicate(std::move(name), std::move(f), c);
-    }
+    auto predicate(std::string name, is_predicate_like auto f, concurrency c = concurrency::serial);
 
     /// @brief Registers a provider node.
-    auto provide(std::string name, is_provider_like auto f, concurrency c = concurrency::serial)
-    {
-      return create_glue().provide(std::move(name), std::move(f), c);
-    }
+    auto provide(std::string name, is_provider_like auto f, concurrency c = concurrency::serial);
 
     /// @brief Registers a transform node.
-    auto transform(std::string name, is_transform_like auto f, concurrency c = concurrency::serial)
-    {
-      return create_glue().transform(std::move(name), std::move(f), c);
-    }
+    auto transform(std::string name, is_transform_like auto f, concurrency c = concurrency::serial);
 
     /// @brief Registers an unfold node.
     template <typename Splitter>
     auto unfold(std::string name,
                 is_predicate_like auto pred,
                 auto unf,
                 std::string destination_data_layer,
-                concurrency c = concurrency::serial)
-    {
-      return glue<Splitter>{graph_, nodes_, nullptr, errors_, config_}.unfold(
-        std::move(name), std::move(pred), std::move(unf), c, std::move(destination_data_layer));
-    }
+                concurrency c = concurrency::serial);
 
     /// @brief Registers a source (used by the framework to create provider nodes)
     template <std::derived_from<source> Source, typename... Args>
     void add_source(std::string name, Args&&... args)
-      requires(not is_bound_object<T>)
-    {
-      // The bound object is created when invoking source<Source>(...), so we explicitly indicate that
-      // no bound object should be used in the create_glue(...) call.
-      return create_glue(false).template add_source<Source>(std::move(name),
-                                                            std::forward<Args>(args)...);
-    }
+      requires(not is_bound_object<T>);
 
     /// @brief Registers an output node.
-    auto output(std::string name, is_output_like auto f, concurrency c = concurrency::serial)
-    {
-      return create_glue().output(std::move(name), std::move(f), c);
-    }
+    auto output(std::string name, is_output_like auto f, concurrency c = concurrency::serial);
 
   protected:
     template <template <typename> typename Proxy, typename U, typename... Args>
     Proxy<U> bind_to(Args&&... args)
-      requires(not is_bound_object<T>)
-    {
-      return Proxy<U>{
-        config_, graph_, nodes_, std::make_shared<U>(std::forward<Args>(args)...), errors_};
-    }
+      requires(not is_bound_object<T>);
 
     graph_proxy(configuration const* config,
                 tbb::flow::graph& g,
                 node_catalog& nodes,
                 std::shared_ptr<T> bound_obj,
                 std::vector<std::string>& errors)
-      requires(is_bound_object<T>)
-      : config_{config}, graph_{g}, nodes_{nodes}, bound_obj_{bound_obj}, errors_{errors}
-    {
-    }
+      requires(is_bound_object<T>);
 
   private:
-    glue<T> create_glue(bool use_bound_object = true)
-    {
-      return glue{graph_, nodes_, (use_bound_object ? bound_obj_ : nullptr), errors_, config_};
-    }
+    glue<T> create_glue(bool use_bound_object = true);
 
     configuration const* config_;
     // Non-owning references to framework-owned resources; graph_proxy<T> is a
@@ -157,6 +109,115 @@ namespace phlex::detail {
     std::shared_ptr<T> bound_obj_;
     std::vector<std::string>& errors_; // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members)
   };
+
+  template <typename T>
+  graph_proxy<T>::graph_proxy(configuration const& config,
+                              tbb::flow::graph& g,
+                              node_catalog& nodes,
+                              std::vector<std::string>& errors)
+    requires(not is_bound_object<T>)
+    : config_{&config}, graph_{g}, nodes_{nodes}, errors_{errors}
+  {
+  }
+
+  template <typename T>
+  template <typename U, typename... Args>
+  graph_proxy<U> graph_proxy<T>::make(Args&&... args)
+    requires(not is_bound_object<T>)
+  {
+    return bind_to<graph_proxy, U>(std::forward<Args>(args)...);
+  }
+
+  template <typename T>
+  template <typename... InitArgs>
+  auto graph_proxy<T>::fold(std::string name,
+                            is_fold_like auto f,
+                            concurrency c,
+                            std::string partition,
+                            InitArgs&&... init_args)
+  {
+    return create_glue().fold(
+      std::move(name), std::move(f), c, std::move(partition), std::forward<InitArgs>(init_args)...);
+  }
+
+  template <typename T>
+  auto graph_proxy<T>::observe(std::string name, is_observer_like auto f, concurrency c)
+  {
+    return create_glue().observe(std::move(name), std::move(f), c);
+  }
+
+  template <typename T>
+  auto graph_proxy<T>::predicate(std::string name, is_predicate_like auto f, concurrency c)
+  {
+    return create_glue().predicate(std::move(name), std::move(f), c);
+  }
+
+  template <typename T>
+  auto graph_proxy<T>::provide(std::string name, is_provider_like auto f, concurrency c)
+  {
+    return create_glue().provide(std::move(name), std::move(f), c);
+  }
+
+  template <typename T>
+  auto graph_proxy<T>::transform(std::string name, is_transform_like auto f, concurrency c)
+  {
+    return create_glue().transform(std::move(name), std::move(f), c);
+  }
+
+  template <typename T>
+  template <typename Splitter>
+  auto graph_proxy<T>::unfold(std::string name,
+                              is_predicate_like auto pred,
+                              auto unf,
+                              std::string destination_data_layer,
+                              concurrency c)
+  {
+    return glue<Splitter>{graph_, nodes_, nullptr, errors_, config_}.unfold(
+      std::move(name), std::move(pred), std::move(unf), c, std::move(destination_data_layer));
+  }
+
+  template <typename T>
+  template <std::derived_from<source> Source, typename... Args>
+  void graph_proxy<T>::add_source(std::string name, Args&&... args)
+    requires(not is_bound_object<T>)
+  {
+    // The bound object is created when invoking add_source<Source>(...), so we explicitly indicate that
+    // no bound object should be used in the create_glue(...) call.
+    return create_glue(false).template add_source<Source>(std::move(name),
+                                                          std::forward<Args>(args)...);
+  }
+
+  template <typename T>
+  auto graph_proxy<T>::output(std::string name, is_output_like auto f, concurrency c)
+  {
+    return create_glue().output(std::move(name), std::move(f), c);
+  }
+
+  template <typename T>
+  template <template <typename> typename Proxy, typename U, typename... Args>
+  Proxy<U> graph_proxy<T>::bind_to(Args&&... args)
+    requires(not is_bound_object<T>)
+  {
+    return Proxy<U>{
+      config_, graph_, nodes_, std::make_shared<U>(std::forward<Args>(args)...), errors_};
+  }
+
+  template <typename T>
+  graph_proxy<T>::graph_proxy(configuration const* config,
+                              tbb::flow::graph& g,
+                              node_catalog& nodes,
+                              std::shared_ptr<T> bound_obj,
+                              std::vector<std::string>& errors)
+    requires(is_bound_object<T>)
+    : config_{config}, graph_{g}, nodes_{nodes}, bound_obj_{bound_obj}, errors_{errors}
+  {
+  }
+
+  template <typename T>
+  glue<T> graph_proxy<T>::create_glue(bool use_bound_object)
+  {
+    return glue{graph_, nodes_, (use_bound_object ? bound_obj_ : nullptr), errors_, config_};
+  }
 }
 
 #endif // PHLEX_CORE_GRAPH_PROXY_HPP