Add config option to enable/disable dynamic plugin reload feature

doc/admin-guide/files/records.config.en.rst

@@ -3988,6 +3988,11 @@ Plug-in Configuration
 
    Specifies the location of |TS| plugins.
 
+.. ts:cv:: CONFIG proxy.config.plugin.dynamic_reload_mode INT 1
+
+   Enables (``1``) or disables (``0``) the dynamic reload feature for remap
+   plugins (`remap.config`). Global plugins (`plugin.config`) do not have dynamic reload feature yet.
+
 SOCKS Processor
 ===============
 

doc/developer-guide/design-documents/reloading-plugins.en.rst

@@ -30,6 +30,8 @@ configuration reload, i.e.::
 
   traffic_ctl config reload
 
+This feature is enabled by default. It can be turned off by setting the configuration variable :ts:cv:`proxy.config.plugin.dynamic_reload_mode` to ``0`` in :file:`records.config`. When the feature is turned off, once ATS is started one will be able load only one version of a plugin, and re-loading the same plugin would do nothing.
+
 Although plugin reloading should be transparent to plugin developers, the following are some design considerations
 and implementation details for this feature.
 
@@ -175,3 +177,7 @@ be reused by 'global' plugins in the future.
 To make sure plugins are still loaded while their code is still in use there is reference counting done around ``PluginDso``
 which inherits ``RefCountObj`` and implements ``acquire()`` and ``release()`` methods which are called by ``TSCreateCont``,
 ``TSVConnCreate`` and ``TSDestroyCont``.
+
+Other notes
+-----------
+When this feature for dynamic plugin reload is turned on (:ts:cv:`proxy.config.plugin.dynamic_reload_mode` is set to ``1``), there is one pitfall users should be aware of. Since "global" plugins do not support this feature while the "remap" plugins do, if a plugin is used as a global plugin as well as a remap plugin, there will be two different copies of the plugin loaded in memory with no state shared between them.

mgmt/RecordsConfig.cc

@@ -1050,6 +1050,8 @@ static const RecordElement RecordsConfig[] =
   ,
   {RECT_CONFIG, "proxy.config.url_remap.pristine_host_hdr", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
   ,
+  {RECT_CONFIG, "proxy.config.plugin.dynamic_reload_mode", RECD_INT, "1", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
+  ,
 
   //##############################################################################
   //#

proxy/Plugin.cc

@@ -34,6 +34,33 @@
 
 #define MAX_PLUGIN_ARGS 64
 
+static PluginDynamicReloadMode plugin_dynamic_reload_mode = REMAP_PLUGIN_DYNAMIC_RELOAD_ON;
+
+bool
+isPluginDynamicReloadEnabled()
+{
+  return plugin_dynamic_reload_mode == REMAP_PLUGIN_DYNAMIC_RELOAD_ON;
+}
+
+void
+parsePluginDynamicReloadConfig()
+{
+  int int_plugin_dynamic_reload_mode;
+  REC_ReadConfigInteger(int_plugin_dynamic_reload_mode, "proxy.config.plugin.dynamic_reload_mode");
+  plugin_dynamic_reload_mode = (PluginDynamicReloadMode)int_plugin_dynamic_reload_mode;
+  if (plugin_dynamic_reload_mode < PLUGIN_DYNAMIC_RELOAD_MODE_MIN || plugin_dynamic_reload_mode > PLUGIN_DYNAMIC_RELOAD_MODE_MAX) {
+    Warning("proxy.config.plugin.dynamic_reload_mode out of range. using default value.");
+    plugin_dynamic_reload_mode = REMAP_PLUGIN_DYNAMIC_RELOAD_ON;
+  }
+  Note("Initialized plugin_dynamic_reload_mode: %d", plugin_dynamic_reload_mode);
+}
+
+void
+parsePluginConfig()
+{
+  parsePluginDynamicReloadConfig();
+}
+
 static const char *plugin_dir = ".";
 
 using init_func_t = void (*)(int, char **);

proxy/Plugin.h

@@ -26,6 +26,18 @@
 #include <string>
 #include "tscore/List.h"
 
+enum PluginDynamicReloadMode {
+  PLUGIN_DYNAMIC_RELOAD_MODE_MIN = 0,
+  PLUGIN_DYNAMIC_RELOAD_OFF      = 0,
+  REMAP_PLUGIN_DYNAMIC_RELOAD_ON = 1,
+  PLUGIN_DYNAMIC_RELOAD_MODE_MAX = 1
+};
+
+// read records.config to parse plugin related configs
+void parsePluginConfig();
+
+bool isPluginDynamicReloadEnabled();
+
 struct PluginRegInfo {
   PluginRegInfo();
   ~PluginRegInfo();

proxy/http/remap/PluginDso.cc

@@ -41,6 +41,8 @@
 PluginDso::PluginDso(const fs::path &configPath, const fs::path &effectivePath, const fs::path &runtimePath)
   : _configPath(configPath), _effectivePath(effectivePath), _runtimePath(runtimePath)
 {
+  PluginDebug(_tag, "PluginDso (%p) created _configPath: [%s] _effectivePath: [%s] _runtimePath: [%s]", this, _configPath.c_str(),
+              _effectivePath.c_str(), _runtimePath.c_str());
 }
 
 PluginDso::~PluginDso()
@@ -71,9 +73,9 @@ PluginDso::load(std::string &error)
   } else {
     PluginDebug(_tag, "plugin '%s' effective path: %s", _configPath.c_str(), _effectivePath.c_str());
 
-    /* Copy the installed plugin DSO to a runtime directory */
+    /* Copy the installed plugin DSO to a runtime directory if dynamic reload enabled */
     std::error_code ec;
-    if (!copy(_effectivePath, _runtimePath, ec)) {
+    if (isDynamicReloadEnabled() && !copy(_effectivePath, _runtimePath, ec)) {
       std::string temp_error;
       temp_error.append("failed to create a copy: ").append(strerror(ec.value()));
       error.assign(temp_error);
@@ -215,6 +217,18 @@ PluginDso::modTime() const
   return _mtime;
 }
 
+/**
+ * @brief file handle returned by dlopen syscall
+ *
+ * @return dlopen filehandle
+ */
+
+void *
+PluginDso::dlOpenHandle() const
+{
+  return _dlh;
+}
+
 /**
  * @brief clean files created by the plugin instance and handle errors
  *
@@ -224,6 +238,10 @@ PluginDso::modTime() const
 void
 PluginDso::clean(std::string &error)
 {
+  if (!isDynamicReloadEnabled()) {
+    return;
+  }
+
   if (false == remove(_runtimePath, _errorCode)) {
     error.append("failed to remove runtime copy: ").append(_errorCode.message());
   }
@@ -266,6 +284,12 @@ PluginDso::instanceCount()
   return _instanceCount.refcount();
 }
 
+bool
+PluginDso::isDynamicReloadEnabled() const
+{
+  return (_runtimePath != _effectivePath);
+}
+
 void
 PluginDso::LoadedPlugins::add(PluginDso *plugin)
 {
@@ -283,8 +307,13 @@ PluginDso::LoadedPlugins::remove(PluginDso *plugin)
   this_ethread()->schedule_imm(new DeleterContinuation<PluginDso>(plugin));
 }
 
+/* check if need to reload the plugin DSO
+ * if dynamic reload not enabled: check if plugin Dso with same effective path aready loaded
+ * if dynamic reload enabled: check if plugin Dso with same effective path and same time stamp already loaded
+ * return pointer to already loaded plugin if found, else return null
+ */
 PluginDso *
-PluginDso::LoadedPlugins::findByEffectivePath(const fs::path &path)
+PluginDso::LoadedPlugins::findByEffectivePath(const fs::path &path, bool dynamicReloadEnabled)
 {
   struct stat sb;
   time_t mtime = 0;
@@ -295,7 +324,8 @@ PluginDso::LoadedPlugins::findByEffectivePath(const fs::path &path)
   SCOPED_MUTEX_LOCK(lock, _mutex, this_ethread());
 
   auto spot = std::find_if(_list.begin(), _list.end(), [&](PluginDso const &plugin) -> bool {
-    return (0 == path.string().compare(plugin.effectivePath().string()) && (mtime == plugin.modTime()));
+    return ((!dynamicReloadEnabled || (mtime == plugin.modTime())) &&
+            (0 == path.string().compare(plugin.effectivePath().string())));
   });
   return spot == _list.end() ? nullptr : static_cast<PluginDso *>(spot);
 }

proxy/http/remap/PluginDso.h

@@ -43,6 +43,8 @@ namespace fs = ts::file;
 #include "I_EventSystem.h"
 #include "tscpp/util/IntrusiveDList.h"
 
+#include "Plugin.h"
+
 class PluginThreadContext : public RefCountObj
 {
 public:
@@ -69,6 +71,7 @@ class PluginDso : public PluginThreadContext
   const fs::path &effectivePath() const;
   const fs::path &runtimePath() const;
   time_t modTime() const;
+  void *dlOpenHandle() const;
 
   /* List used by the plugin factory */
   using self_type  = PluginDso; ///< Self reference type.
@@ -89,14 +92,15 @@ class PluginDso : public PluginThreadContext
   void incInstanceCount();
   void decInstanceCount();
   int instanceCount();
+  bool isDynamicReloadEnabled() const;
 
   class LoadedPlugins : public RefCountObj
   {
   public:
     LoadedPlugins() : _mutex(new_ProxyMutex()) {}
     void add(PluginDso *plugin);
     void remove(PluginDso *plugin);
-    PluginDso *findByEffectivePath(const fs::path &path);
+    PluginDso *findByEffectivePath(const fs::path &path, bool dynamicReloadEnabled);
     void indicatePreReload(const char *factoryId);
     void indicatePostReload(bool reloadSuccessful, const std::unordered_map<PluginDso *, int> &pluginUsed, const char *factoryId);
 

proxy/http/remap/PluginFactory.cc

@@ -141,7 +141,7 @@ PluginFactory::getUuid()
  * @return pointer to a plugin instance, nullptr if failure
  */
 RemapPluginInst *
-PluginFactory::getRemapPlugin(const fs::path &configPath, int argc, char **argv, std::string &error)
+PluginFactory::getRemapPlugin(const fs::path &configPath, int argc, char **argv, std::string &error, bool dynamicReloadEnabled)
 {
   /* Discover the effective path by looking into the search dirs */
   fs::path effectivePath = getEffectivePath(configPath);
@@ -153,21 +153,31 @@ PluginFactory::getRemapPlugin(const fs::path &configPath, int argc, char **argv,
   }
 
   /* Only one plugin with this effective path can be loaded by a plugin factory */
-  RemapPluginInfo *plugin = dynamic_cast<RemapPluginInfo *>(findByEffectivePath(effectivePath));
+  RemapPluginInfo *plugin = dynamic_cast<RemapPluginInfo *>(findByEffectivePath(effectivePath, dynamicReloadEnabled));
   RemapPluginInst *inst   = nullptr;
 
   if (nullptr == plugin) {
     /* The plugin requested have not been loaded yet. */
     PluginDebug(_tag, "plugin '%s' has not been loaded yet, loading as remap plugin", configPath.c_str());
 
     fs::path runtimePath;
-    runtimePath /= _runtimeDir;
-    runtimePath /= effectivePath.relative_path();
 
-    fs::path parent = runtimePath.parent_path();
-    if (!fs::create_directories(parent, _ec)) {
-      error.assign("failed to create plugin runtime dir");
-      return nullptr;
+    // if dynamic reload enabled then create a temporary location to copy .so and load from there
+    // else load from original location
+    if (dynamicReloadEnabled) {
+      runtimePath /= _runtimeDir;
+      runtimePath /= effectivePath.relative_path();
+
+      fs::path parent = runtimePath.parent_path();
+      PluginDebug(_tag, "Using effectivePath: [%s] runtimePath: [%s] parent: [%s]", effectivePath.c_str(), runtimePath.c_str(),
+                  parent.c_str());
+      if (!fs::create_directories(parent, _ec)) {
+        error.assign("failed to create plugin runtime dir");
+        return nullptr;
+      }
+    } else {
+      runtimePath = effectivePath;
+      PluginDebug(_tag, "Using effectivePath: [%s] runtimePath: [%s]", effectivePath.c_str(), runtimePath.c_str());
     }
 
     plugin = new RemapPluginInfo(configPath, effectivePath, runtimePath);
@@ -187,7 +197,7 @@ PluginFactory::getRemapPlugin(const fs::path &configPath, int argc, char **argv,
           delete plugin;
         }
 
-        if (_preventiveCleaning) {
+        if (dynamicReloadEnabled && _preventiveCleaning) {
           clean(error);
         }
       } else {
@@ -244,9 +254,9 @@ PluginFactory::getEffectivePath(const fs::path &configPath)
  * @return plugin found or nullptr if not found
  */
 PluginDso *
-PluginFactory::findByEffectivePath(const fs::path &path)
+PluginFactory::findByEffectivePath(const fs::path &path, bool dynamicReloadEnabled)
 {
-  return PluginDso::loadedPlugins()->findByEffectivePath(path);
+  return PluginDso::loadedPlugins()->findByEffectivePath(path, dynamicReloadEnabled);
 }
 
 /**

proxy/http/remap/PluginFactory.h

@@ -94,7 +94,7 @@ class PluginFactory
   PluginFactory &setRuntimeDir(const fs::path &runtimeDir);
   PluginFactory &addSearchDir(const fs::path &searchDir);
 
-  RemapPluginInst *getRemapPlugin(const fs::path &configPath, int argc, char **argv, std::string &error);
+  RemapPluginInst *getRemapPlugin(const fs::path &configPath, int argc, char **argv, std::string &error, bool dynamicReloadEnabled);
 
   virtual const char *getUuid();
   void clean(std::string &error);
@@ -104,7 +104,7 @@ class PluginFactory
   void indicatePostReload(bool reloadSuccessful);
 
 protected:
-  PluginDso *findByEffectivePath(const fs::path &path);
+  PluginDso *findByEffectivePath(const fs::path &path, bool dynamicReloadEnabled);
   fs::path getEffectivePath(const fs::path &configPath);
 
   std::vector<fs::path> _searchDirs; /** @brief ordered list of search paths where we look for plugins */

proxy/http/remap/RemapConfig.cc

@@ -817,7 +817,8 @@ remap_load_plugin(const char **argv, int argc, url_mapping *mp, char *errbuf, in
     REC_ReadConfigInteger(elevate_access, "proxy.config.plugin.load_elevated");
     ElevateAccess access(elevate_access ? ElevateAccess::FILE_PRIVILEGE : 0);
 
-    pi = rewrite->pluginFactory.getRemapPlugin(ts::file::path(const_cast<const char *>(c)), parc, pargv, error);
+    pi = rewrite->pluginFactory.getRemapPlugin(ts::file::path(const_cast<const char *>(c)), parc, pargv, error,
+                                               isPluginDynamicReloadEnabled());
   } // done elevating access
 
   bool result = true;

proxy/http/remap/unit-tests/plugin_testing_common.cc

@@ -50,3 +50,24 @@ getTemporaryDir()
 
   return fs::path(mkdtemp(dirNameTemplate));
 }
+
+// implement functions to support unit-testing of option to enable/disable dynamic reload of plugins
+static PluginDynamicReloadMode plugin_dynamic_reload_mode = REMAP_PLUGIN_DYNAMIC_RELOAD_ON;
+
+bool
+isPluginDynamicReloadEnabled()
+{
+  return plugin_dynamic_reload_mode == REMAP_PLUGIN_DYNAMIC_RELOAD_ON;
+}
+
+void
+enablePluginDynamicReload()
+{
+  plugin_dynamic_reload_mode = REMAP_PLUGIN_DYNAMIC_RELOAD_ON;
+}
+
+void
+disablePluginDynamicReload()
+{
+  plugin_dynamic_reload_mode = PLUGIN_DYNAMIC_RELOAD_OFF;
+}

proxy/http/remap/unit-tests/plugin_testing_common.h

@@ -101,3 +101,7 @@ void PrintToStdErr(const char *fmt, ...);
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
+
+// functions to support unit-testing of option to enable/disable dynamic reload of plugins
+void enablePluginDynamicReload();
+void disablePluginDynamicReload();