diff --git a/cmake/layout.cmake b/cmake/layout.cmake index 69a7b239705..72dcbf0fd7c 100644 --- a/cmake/layout.cmake +++ b/cmake/layout.cmake @@ -43,7 +43,7 @@ set(CMAKE_INSTALL_LOCALSTATEDIR CACHE STRING "localstatedir" ) set(CMAKE_INSTALL_RUNSTATEDIR - "${CMAKE_INSTALL_LOCALSTATEDIR}/trafficserver" + "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LOCALSTATEDIR}/trafficserver" CACHE STRING "runstatedir" ) set(CMAKE_INSTALL_DATAROOTDIR diff --git a/include/proxy/http/remap/PluginDso.h b/include/proxy/http/remap/PluginDso.h index e7c84e97c9e..d3ea8087a2b 100644 --- a/include/proxy/http/remap/PluginDso.h +++ b/include/proxy/http/remap/PluginDso.h @@ -170,12 +170,8 @@ class PluginDso : public PluginThreadContext static constexpr const char *const _tag = "plugin_dso"; /** @brief log tag used by this class */ static const DbgCtl &_dbg_ctl(); swoc::file::file_time_type _mtime{fs::file_time_type::min()}; /* @brief modification time of the DSO's file, used for checking */ - bool _preventiveCleaning = true; - - static Ptr - _plugins; /** @brief a global list of plugins, usually maintained by a plugin factory or plugin instance itself */ - std::atomic _instanceCount{ - 0}; /** @brief used for properly calling "done" and "indicate config reload" methods by the factory */ + static Ptr _plugins; /** @brief a global list of plugins */ + std::atomic _instanceCount{0}; /** @brief used for properly calling "done" and "indicate config reload" */ }; inline const DbgCtl & diff --git a/include/proxy/http/remap/PluginFactory.h b/include/proxy/http/remap/PluginFactory.h index 9007c80c9cf..fd08c00000f 100644 --- a/include/proxy/http/remap/PluginFactory.h +++ b/include/proxy/http/remap/PluginFactory.h @@ -99,12 +99,13 @@ class PluginFactory RemapPluginInst *getRemapPlugin(const fs::path &configPath, int argc, char **argv, std::string &error, bool dynamicReloadEnabled); virtual const char *getUuid(); - void clean(std::string &error); void deactivate(); void indicatePreReload(); void indicatePostReload(bool reloadSuccessful); + static void cleanup(); // For startup, clean out all temporary directory we may have left from before + protected: PluginDso *findByEffectivePath(const fs::path &path, bool dynamicReloadEnabled); fs::path getEffectivePath(const fs::path &configPath); @@ -117,7 +118,6 @@ class PluginFactory ATSUuid *_uuid = nullptr; std::error_code _ec; - bool _preventiveCleaning = true; static constexpr const char *const _tag = "plugin_factory"; /** @brief log tag used by this class */ static const DbgCtl &_dbg_ctl(); diff --git a/src/proxy/http/remap/PluginDso.cc b/src/proxy/http/remap/PluginDso.cc index 6c23823687d..cd0499d8e60 100644 --- a/src/proxy/http/remap/PluginDso.cc +++ b/src/proxy/http/remap/PluginDso.cc @@ -68,6 +68,7 @@ PluginDso::PluginDso(const fs::path &configPath, const fs::path &effectivePath, PluginDso::~PluginDso() { std::string error; + (void)unload(error); } @@ -136,11 +137,6 @@ PluginDso::load(std::string &error, const fs::path &compilerPath) PluginError("plugin '%s' failed to load: %s", _configPath.c_str(), error.c_str()); } } - - /* Remove the runtime DSO copy even if we succeed loading to avoid leftovers after crashes */ - if (_preventiveCleaning) { - clean(error); - } } PluginDbg(_dbg_ctl(), "plugin '%s' finished loading DSO", _configPath.c_str()); diff --git a/src/proxy/http/remap/PluginFactory.cc b/src/proxy/http/remap/PluginFactory.cc index 3f13543a71d..c5217cee1aa 100644 --- a/src/proxy/http/remap/PluginFactory.cc +++ b/src/proxy/http/remap/PluginFactory.cc @@ -36,6 +36,7 @@ #include "../../../iocore/eventsystem/P_EventSystem.h" #include /* std::swap */ +#include RemapPluginInst::RemapPluginInst(RemapPluginInfo &plugin) : _plugin(plugin) { @@ -102,7 +103,16 @@ PluginFactory::~PluginFactory() _instList.apply([](RemapPluginInst *pluginInst) -> void { delete pluginInst; }); _instList.clear(); - fs::remove_all(_runtimeDir, _ec); + if (!TSSystemState::is_event_system_shut_down()) { + uint32_t elevate_access = 0; + + REC_ReadConfigInteger(elevate_access, "proxy.config.plugin.load_elevated"); + ElevateAccess access(elevate_access ? ElevateAccess::FILE_PRIVILEGE : 0); + + fs::remove_all(_runtimeDir, _ec); + } else { + fs::remove_all(_runtimeDir, _ec); // Try anyways + } PluginDbg(_dbg_ctl(), "destroyed plugin factory %s", getUuid()); delete _uuid; @@ -226,9 +236,6 @@ PluginFactory::getRemapPlugin(const fs::path &configPath, int argc, char **argv, delete plugin; } - if (dynamicReloadEnabled && _preventiveCleaning) { - clean(error); - } } else { /* Plugin DSO load failed. */ PluginDbg(_dbg_ctl(), "plugin '%s' DSO load failed", configPath.c_str()); @@ -276,6 +283,30 @@ PluginFactory::getEffectivePath(const fs::path &configPath) return path; } +void +PluginFactory::cleanup() +{ + std::error_code ec; + std::string path(RecConfigReadRuntimeDir()); + + try { + if (path.starts_with("/") && std::filesystem::is_directory(path)) { + for (const auto &entry : std::filesystem::directory_iterator(path, ec)) { + if (entry.is_directory()) { + std::string dir_name = entry.path().string(); + int dash_count = std::count(dir_name.begin(), dir_name.end(), '-'); // All UUIDs have 4 dashes + + if (dash_count == 4) { + std::filesystem::remove_all(dir_name, ec); + } + } + } + } + } catch (std::exception &e) { + PluginError("Error cleaning up runtime directory: %s", e.what()); + } +} + /** * @brief Find a plugin by path from our linked plugin list by using plugin effective (canonical) path * @@ -326,9 +357,3 @@ PluginFactory::indicatePostReload(bool reloadSuccessful) PluginDso::loadedPlugins()->indicatePostReload(reloadSuccessful, pluginUsed, getUuid()); } - -void -PluginFactory::clean(std::string & /* error ATS_UNUSED */) -{ - fs::remove_all(_runtimeDir, _ec); -} diff --git a/src/proxy/http/remap/unit-tests/test_PluginDso.cc b/src/proxy/http/remap/unit-tests/test_PluginDso.cc index 72eb31f33ef..e3d40714942 100644 --- a/src/proxy/http/remap/unit-tests/test_PluginDso.cc +++ b/src/proxy/http/remap/unit-tests/test_PluginDso.cc @@ -66,8 +66,6 @@ class PluginDsoUnitTest : public PluginDso PluginDsoUnitTest(const fs::path &configPath, const fs::path &effectivePath, const fs::path &runtimePath) : PluginDso(configPath, effectivePath, runtimePath) { - /* don't remove runtime DSO copy preventively so we can check if it was created properly */ - _preventiveCleaning = false; } virtual void diff --git a/src/proxy/http/remap/unit-tests/test_PluginFactory.cc b/src/proxy/http/remap/unit-tests/test_PluginFactory.cc index 9053d9668f0..6997da4d348 100644 --- a/src/proxy/http/remap/unit-tests/test_PluginFactory.cc +++ b/src/proxy/http/remap/unit-tests/test_PluginFactory.cc @@ -68,11 +68,7 @@ static fs::path tempComponent = fs::path("c71e2bab-90dc-4770-9535-c9304c3de38e") class PluginFactoryUnitTest : public PluginFactory { public: - PluginFactoryUnitTest(const fs::path &tempComponent) - { - _tempComponent = tempComponent; - _preventiveCleaning = false; - } + PluginFactoryUnitTest(const fs::path &tempComponent) { _tempComponent = tempComponent; } protected: const char * @@ -305,9 +301,7 @@ SCENARIO("loading plugins", "[plugin][core]") validateSuccessfulConfigPathTest(plugin, error, effectivePath, runtimePath); CHECK(nullptr != PluginDso::loadedPlugins()->findByEffectivePath(effectivePath, isPluginDynamicReloadEnabled())); - // check Dso at effective path still exists while copy at runtime path doesn't CHECK(fs::exists(plugin->_plugin.effectivePath())); - CHECK(!fs::exists(plugin->_plugin.runtimePath())); } teardownConfigPathTest(factory); @@ -606,13 +600,10 @@ checkTwoLoadedVersionsDifferent(const RemapPluginInst *plugin_v1, const RemapPlu // 2 versions can be different only when dynamic reload enabled CHECK(isPluginDynamicReloadEnabled()); - // check Dso at effective path still exists while the copy at runtime path doesn't CHECK(plugin_v1->_plugin.effectivePath() != plugin_v1->_plugin.runtimePath()); CHECK(fs::exists(plugin_v1->_plugin.effectivePath())); - CHECK(!fs::exists(plugin_v1->_plugin.runtimePath())); CHECK(plugin_v2->_plugin.effectivePath() != plugin_v2->_plugin.runtimePath()); CHECK(fs::exists(plugin_v2->_plugin.effectivePath())); - CHECK(!fs::exists(plugin_v2->_plugin.runtimePath())); } void diff --git a/src/traffic_server/traffic_server.cc b/src/traffic_server/traffic_server.cc index 31a4493c852..ad4492c60d8 100644 --- a/src/traffic_server/traffic_server.cc +++ b/src/traffic_server/traffic_server.cc @@ -1767,7 +1767,6 @@ configure_io_uring() // // Main // - int main(int /* argc ATS_UNUSED */, const char **argv) { @@ -1907,6 +1906,9 @@ main(int /* argc ATS_UNUSED */, const char **argv) signal_register_crash_handler(crash_logger_invoke); } + // Clean out any remnant temporary plugin (UUID named) directories. + PluginFactory::cleanup(); + #if TS_USE_POSIX_CAP // Change the user of the process. // Do this before we start threads so we control the user id of the