Merge pull request #2473 from DARMA-tasking/2472-argvcontainer-leaks-c-string-from-strdup

2472 argvcontainer leaks c string from strdup

Author: lifflander

src/vt/collective/collective_ops.cc

@@ -46,6 +46,7 @@
 #include "vt/runtime/runtime.h"
 #include "vt/scheduler/scheduler.h"
 #include "vt/runtime/runtime_inst.h"
+#include "vt/configs/arguments/args.h"
 
 #include <memory>
 #include <cstdlib>
@@ -216,7 +217,7 @@ void printOverwrittens(
 template <runtime::RuntimeInstType instance>
 RuntimePtrType CollectiveAnyOps<instance>::initialize(
   int& argc, char**& argv, bool is_interop, MPI_Comm* comm,
-  arguments::AppConfig const* appConfig
+  arguments::AppConfig const* appConfig, bool print_startup_banner
 ) {
   using vt::runtime::RuntimeInst;
   using vt::runtime::Runtime;
@@ -235,7 +236,7 @@ RuntimePtrType CollectiveAnyOps<instance>::initialize(
     ::vt::rt = rt_ptr;
     curRT = rt_ptr;
   }
-  RuntimeInst<instance>::rt->initialize();
+  RuntimeInst<instance>::rt->initialize(false, print_startup_banner);
 
   // If appConfig is not nullptr, compare CLI arguments with user-defined ones,
   // and report overwritten ones.
@@ -246,6 +247,33 @@ RuntimePtrType CollectiveAnyOps<instance>::initialize(
   return runtime::makeRuntimePtr(rt_ptr);
 }
 
+template <runtime::RuntimeInstType instance>
+/*static*/ RuntimePtrType CollectiveAnyOps<instance>::initializePreconfigured(
+  std::unique_ptr<StartupConfig> startup_config, MPI_Comm* comm,
+  arguments::AppConfig const* app_config, bool print_startup_banner
+) {
+  using vt::runtime::RuntimeInst;
+  using vt::runtime::Runtime;
+  using vt::runtime::eRuntimeInstance;
+
+  MPI_Comm resolved_comm = comm not_eq nullptr ? *comm : MPI_COMM_WORLD;
+
+  RuntimeInst<instance>::rt = std::make_unique<Runtime>(
+    std::move(startup_config), resolved_comm, app_config,
+    eRuntimeInstance::DefaultInstance
+  );
+
+  auto rt_ptr = RuntimeInst<instance>::rt.get();
+  if (instance == runtime::RuntimeInstType::DefaultInstance) {
+    // Set global variable for default instance for backward compatibility
+    ::vt::rt = rt_ptr;
+    curRT = rt_ptr;
+  }
+  RuntimeInst<instance>::rt->initialize(false, print_startup_banner);
+
+  return runtime::makeRuntimePtr(rt_ptr);
+}
+
 template <runtime::RuntimeInstType instance>
 void CollectiveAnyOps<instance>::setCurrentRuntimeTLS(RuntimeUnsafePtrType in) {
   bool const has_rt = in != nullptr;

src/vt/collective/collective_ops.h

@@ -60,10 +60,26 @@ static constexpr runtime::RuntimeInstType const collective_default_inst =
 template <runtime::RuntimeInstType instance = collective_default_inst>
 struct CollectiveAnyOps {
   // The general methods that interact with the managed runtime holder
+
+  /**
+   * \brief Initialize VT
+   *
+   * \param[in] argc (to modify)
+   * \param[in] argv (to modify)
+   * \param[in] is_interop use interop mode (don't initialize MPI)
+   * \param[in] comm optional communicator
+   * \param[in] app_config (optional) base VT configuration to use
+   * \param[in] print_startup_banner (optional) whether to print startup banner
+   *
+   * \return runtime pointer
+   */
   static RuntimePtrType initialize(
     int& argc, char**& argv, bool is_interop = false, MPI_Comm* comm = nullptr,
-    arguments::AppConfig const* appConfig = nullptr
+    arguments::AppConfig const* appConfig = nullptr,
+    bool print_startup_banner = true
   );
+
+  //// Old version of initialize with number of worker threads
   [[deprecated]] static RuntimePtrType initialize(
     int& argc, char**& argv, PhysicalResourceType const /* num_workers */,
     bool is_interop = false, MPI_Comm* comm = nullptr,
@@ -72,6 +88,24 @@ struct CollectiveAnyOps {
   {
     return initialize(argc, argv, is_interop, comm, appConfig);
   }
+
+  /**
+   * \brief Initialize a VT runtime with arguments preconfigured
+   *
+   * \param[in] startup_config startup config returned from \c preconfigure
+   * \param[in] comm the communicator for VT to use
+   * \param[in] app_config the app config for overriding the config
+   * \param[in] print_startup_banner (optional) whether to print startup banner
+   *
+   * \return the runtime pointer
+   */
+  static RuntimePtrType initializePreconfigured(
+    std::unique_ptr<StartupConfig> startup_config,
+    MPI_Comm* comm = nullptr,
+    arguments::AppConfig const* app_config = nullptr,
+    bool print_startup_banner = true
+  );
+
   static void finalize(RuntimePtrType in_rt = nullptr);
   static void scheduleThenFinalize(RuntimePtrType in_rt = nullptr);
   static void setCurrentRuntimeTLS(RuntimeUnsafePtrType in_rt = nullptr);

src/vt/collective/startup.cc

@@ -46,37 +46,39 @@
 #include "vt/collective/collective_ops.h"
 #include "vt/runtime/runtime_headers.h"
 #include "vt/context/context.h"
+#include "vt/configs/arguments/args.h"
 
 namespace vt {
 
-std::unique_ptr<arguments::ArgvContainer>
-preconfigure(int& argc, char**& argv) {
-  return std::make_unique<arguments::ArgvContainer>(argc, argv);
+std::unique_ptr<StartupConfig> preconfigure(
+  int& argc, char**& argv
+) {
+  auto arg_config = std::make_unique<arguments::ArgConfig>();
+  auto parse_input_holder = arg_config->setupInputHolder(argc, argv);
+  return std::make_unique<StartupConfig>(
+    std::move(arg_config), std::move(parse_input_holder)
+  );
 }
 
 RuntimePtrType initializePreconfigured(
-  MPI_Comm* comm, arguments::AppConfig const* appConfig,
-  arguments::ArgvContainer const* preconfigure_args
+  std::unique_ptr<StartupConfig> startup_config,
+  MPI_Comm* comm,
+  arguments::AppConfig const* app_config,
+  bool print_startup_banner
 ) {
-  arguments::ArgvContainer args =
-    preconfigure_args ? *preconfigure_args : arguments::ArgvContainer{};
-
-  auto argc = args.getArgc();
-  auto argv_container = args.getArgvDeepCopy();
-  auto argv = argv_container.get();
-  bool const is_interop = comm != nullptr;
-  return CollectiveOps::initialize(
-    argc, argv, is_interop, comm, appConfig
+  return CollectiveOps::initializePreconfigured(
+    std::move(startup_config), comm, app_config, print_startup_banner
   );
 }
 
 // vt::{initialize,finalize} for main ::vt namespace
 RuntimePtrType initialize(
-  int& argc, char**& argv, MPI_Comm* comm, arguments::AppConfig const* appConfig
+  int& argc, char**& argv, MPI_Comm* comm, arguments::AppConfig const* appConfig,
+  bool print_startup_banner
 ) {
   bool const is_interop = comm != nullptr;
   return CollectiveOps::initialize(
-    argc, argv, is_interop, comm, appConfig
+    argc, argv, is_interop, comm, appConfig, print_startup_banner
   );
 }
 

src/vt/collective/startup.h

@@ -46,28 +46,73 @@
 
 #include "vt/config.h"
 #include "vt/runtime/runtime_headers.h"
-#include "vt/configs/arguments/argv_container.h"
+#include "vt/collective/startup_config.h"
 #include <mpi.h>
 
+#include <memory>
+
 namespace vt {
 
-std::unique_ptr<arguments::ArgvContainer>
-preconfigure(int& argc, char**& argv);
+/**
+ * \brief Preconfigure VT with argc/argv. This will remove all VT arguments and
+ * create a \c StartupConfig for VT that should be passed to \c
+ * initializePreconfigured. Optionally, one may specify an MPI communicator to
+ * use (otherwise, it defaults to \c MPI_COMM_WORLD).
+ *
+ * \note MPI must be initialized to call this function because if an error
+ * occurs it uses MPI rank to limit how many times the error text gets printed.
+ *
+ * \param[in] argc argc (modifies it to remove VT arguments)
+ * \param[in] argv argv (modifies it to remove VT arguments)
+ *
+ * \return the \c StartupConfig to pass to VT
+ */
+std::unique_ptr<StartupConfig> preconfigure(
+  int& argc, char**& argv
+);
+
+/**
+ * \brief Initialize VT after it has been preconfigured
+ *
+ * \param[in] startup_config the arg config
+ * \param[in] comm optional communicator
+ * \param[in] app_config (optional) base VT configuration to use
+ * \param[in] print_startup_banner (optional) whether to print startup banner
+ *
+ * \return the runtime pointer
+ */
 RuntimePtrType initializePreconfigured(
+  std::unique_ptr<StartupConfig> startup_config,
   MPI_Comm* comm = nullptr,
-  arguments::AppConfig const* appConfig = nullptr,
-  arguments::ArgvContainer const* preconfigure_args = nullptr);
+  arguments::AppConfig const* app_config = nullptr,
+  bool print_startup_banner = true
+);
 
+/**
+ * \brief Initialize VT
+ *
+ * \param[in] argc (to modify)
+ * \param[in] argv (to modify)
+ * \param[in] comm optional communicator
+ * \param[in] app_config (optional) base VT configuration to use
+ * \param[in] print_startup_banner (optional) whether to print startup banner
+ *
+ * \return the runtime pointer
+ */
 RuntimePtrType initialize(
   int& argc, char**& argv, MPI_Comm* comm = nullptr,
-  arguments::AppConfig const* appConfig = nullptr
+  arguments::AppConfig const* appConfig = nullptr,
+  bool print_startup_banner = true
 );
+
 RuntimePtrType initialize(MPI_Comm* comm = nullptr);
+
 RuntimePtrType initialize(
   int& argc, char**& argv, arguments::AppConfig const* appConfig
 );
 
 void finalize(RuntimePtrType in_rt);
+
 void finalize();
 
 } /* end namespace vt */

src/vt/collective/startup_config.cc

@@ -0,0 +1,58 @@
+/*
+//@HEADER
+// *****************************************************************************
+//
+//                              startup_config.cc
+//                       DARMA/vt => Virtual Transport
+//
+// Copyright 2019-2024 National Technology & Engineering Solutions of Sandia, LLC
+// (NTESS). Under the terms of Contract DE-NA0003525 with NTESS, the U.S.
+// Government retains certain rights in this software.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+// * Redistributions of source code must retain the above copyright notice,
+//   this list of conditions and the following disclaimer.
+//
+// * Redistributions in binary form must reproduce the above copyright notice,
+//   this list of conditions and the following disclaimer in the documentation
+//   and/or other materials provided with the distribution.
+//
+// * Neither the name of the copyright holder nor the names of its
+//   contributors may be used to endorse or promote products derived from this
+//   software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+// POSSIBILITY OF SUCH DAMAGE.
+//
+// Questions? Contact [email protected]
+//
+// *****************************************************************************
+//@HEADER
+*/
+
+#include "vt/collective/startup_config.h"
+#include "vt/configs/arguments/args.h"
+
+namespace vt {
+
+StartupConfig::StartupConfig(
+  std::unique_ptr<arguments::ArgConfig> in_arg_config,
+  std::unique_ptr<ParseInputHolder> in_parse_input_holder
+) : arg_config_(std::move(in_arg_config)),
+    parse_input_holder_(std::move(in_parse_input_holder))
+{ }
+
+StartupConfig::~StartupConfig() = default;
+
+} /* end namespace vt */