From 0c43c08d6f2a0a46a66748b6322673ed8ba4e74d Mon Sep 17 00:00:00 2001 From: Mahmoud Mazouz Date: Thu, 25 Jul 2024 18:36:14 +0200 Subject: [PATCH] Don't load plugins/volumes when `plugins_loading/enabled:false` (#1269) --- commons/zenoh-util/src/lib_loader.rs | 34 ++++++++++--------- .../zenoh-plugin-storage-manager/src/lib.rs | 4 ++- plugins/zenoh-plugin-trait/src/manager.rs | 4 +-- .../src/manager/dynamic_plugin.rs | 20 +++++++---- .../src/manager/static_plugin.rs | 4 +-- zenoh/src/net/runtime/adminspace.rs | 11 +++++- 6 files changed, 48 insertions(+), 29 deletions(-) diff --git a/commons/zenoh-util/src/lib_loader.rs b/commons/zenoh-util/src/lib_loader.rs index a2fb98da23..9d4a52c332 100644 --- a/commons/zenoh-util/src/lib_loader.rs +++ b/commons/zenoh-util/src/lib_loader.rs @@ -20,7 +20,7 @@ use std::{ use libloading::Library; use tracing::{debug, warn}; -use zenoh_core::zconfigurable; +use zenoh_core::{zconfigurable, zerror}; use zenoh_result::{bail, ZResult}; zconfigurable! { @@ -35,15 +35,13 @@ zconfigurable! { /// LibLoader allows search for libraries and to load them. #[derive(Clone, Debug)] pub struct LibLoader { - search_paths: Vec, + search_paths: Option>, } impl LibLoader { /// Return an empty `LibLoader`. pub fn empty() -> LibLoader { - LibLoader { - search_paths: Vec::new(), - } + LibLoader { search_paths: None } } /// Returns the list of search paths used by `LibLoader::default()` @@ -83,12 +81,14 @@ impl LibLoader { } } - LibLoader { search_paths } + LibLoader { + search_paths: Some(search_paths), + } } /// Return the list of search paths used by this [LibLoader] - pub fn search_paths(&self) -> &[PathBuf] { - &self.search_paths + pub fn search_paths(&self) -> Option<&[PathBuf]> { + self.search_paths.as_deref() } /// Load a library from the specified path. @@ -118,7 +118,7 @@ impl LibLoader { /// /// This function calls [libloading::Library::new()](https://docs.rs/libloading/0.7.0/libloading/struct.Library.html#method.new) /// which is unsafe. - pub unsafe fn search_and_load(&self, name: &str) -> ZResult<(Library, PathBuf)> { + pub unsafe fn search_and_load(&self, name: &str) -> ZResult> { let filename = format!("{}{}{}", *LIB_PREFIX, name, *LIB_SUFFIX); let filename_ostr = OsString::from(&filename); tracing::debug!( @@ -126,13 +126,16 @@ impl LibLoader { filename, self.search_paths ); - for dir in &self.search_paths { + let Some(search_paths) = self.search_paths() else { + return Ok(None); + }; + for dir in search_paths { match dir.read_dir() { Ok(read_dir) => { for entry in read_dir.flatten() { if entry.file_name() == filename_ostr { let path = entry.path(); - return Ok((Library::new(path.clone())?, path)); + return Ok(Some((Library::new(path.clone())?, path))); } } } @@ -142,7 +145,7 @@ impl LibLoader { ), } } - bail!("Library file '{}' not found", filename) + Err(zerror!("Library file '{}' not found", filename).into()) } /// Search and load all libraries with filename starting with [struct@LIB_PREFIX]+`prefix` and ending with [struct@LIB_SUFFIX]. @@ -158,7 +161,7 @@ impl LibLoader { pub unsafe fn load_all_with_prefix( &self, prefix: Option<&str>, - ) -> Vec<(Library, PathBuf, String)> { + ) -> Option> { let lib_prefix = format!("{}{}", *LIB_PREFIX, prefix.unwrap_or("")); tracing::debug!( "Search for libraries {}*{} to load in {:?}", @@ -166,9 +169,8 @@ impl LibLoader { *LIB_SUFFIX, self.search_paths ); - let mut result = vec![]; - for dir in &self.search_paths { + for dir in self.search_paths()? { match dir.read_dir() { Ok(read_dir) => { for entry in read_dir.flatten() { @@ -199,7 +201,7 @@ impl LibLoader { ), } } - result + Some(result) } pub fn _plugin_name(path: &std::path::Path) -> Option<&str> { diff --git a/plugins/zenoh-plugin-storage-manager/src/lib.rs b/plugins/zenoh-plugin-storage-manager/src/lib.rs index 3c64e3fe35..3f98725a5e 100644 --- a/plugins/zenoh-plugin-storage-manager/src/lib.rs +++ b/plugins/zenoh-plugin-storage-manager/src/lib.rs @@ -229,7 +229,9 @@ impl StorageRuntimeInner { self.plugins_manager .declare_dynamic_plugin_by_name(volume_id, backend_name, true)? }; - let loaded = declared.load()?; + let loaded = declared + .load()? + .expect("Volumes should not loaded if if the storage-manager plugin is not loaded"); loaded.start(config)?; Ok(()) diff --git a/plugins/zenoh-plugin-trait/src/manager.rs b/plugins/zenoh-plugin-trait/src/manager.rs index 90651532ec..4776aa31a3 100644 --- a/plugins/zenoh-plugin-trait/src/manager.rs +++ b/plugins/zenoh-plugin-trait/src/manager.rs @@ -25,7 +25,7 @@ use crate::*; pub trait DeclaredPlugin: PluginStatus { fn as_status(&self) -> &dyn PluginStatus; - fn load(&mut self) -> ZResult<&mut dyn LoadedPlugin>; + fn load(&mut self) -> ZResult>>; fn loaded(&self) -> Option<&dyn LoadedPlugin>; fn loaded_mut(&mut self) -> Option<&mut dyn LoadedPlugin>; } @@ -88,7 +88,7 @@ impl DeclaredPlugin &dyn PluginStatus { self } - fn load(&mut self) -> ZResult<&mut dyn LoadedPlugin> { + fn load(&mut self) -> ZResult>> { self.0.load() } fn loaded(&self) -> Option<&dyn LoadedPlugin> { diff --git a/plugins/zenoh-plugin-trait/src/manager/dynamic_plugin.rs b/plugins/zenoh-plugin-trait/src/manager/dynamic_plugin.rs index 24c873814e..50bed07a4f 100644 --- a/plugins/zenoh-plugin-trait/src/manager/dynamic_plugin.rs +++ b/plugins/zenoh-plugin-trait/src/manager/dynamic_plugin.rs @@ -13,7 +13,7 @@ use std::path::{Path, PathBuf}; use libloading::Library; -use zenoh_result::{bail, ZResult}; +use zenoh_result::{bail, zerror, ZResult}; use zenoh_util::LibLoader; use crate::*; @@ -28,7 +28,7 @@ pub enum DynamicPluginSource { } impl DynamicPluginSource { - fn load(&self) -> ZResult<(Library, PathBuf)> { + fn load(&self) -> ZResult> { match self { DynamicPluginSource::ByName((libloader, name)) => unsafe { libloader.search_and_load(name) @@ -36,11 +36,11 @@ impl DynamicPluginSource { DynamicPluginSource::ByPaths(paths) => { for path in paths { match unsafe { LibLoader::load_file(path) } { - Ok((l, p)) => return Ok((l, p)), + Ok((l, p)) => return Ok(Some((l, p))), Err(e) => tracing::debug!("Attempt to load {} failed: {}", path, e), } } - bail!("Plugin not found in {:?}", &paths) + Err(zerror!("Plugin not found in {:?}", &paths).into()) } } } @@ -179,16 +179,22 @@ impl DeclaredPlugin &dyn PluginStatus { self } - fn load(&mut self) -> ZResult<&mut dyn LoadedPlugin> { + fn load(&mut self) -> ZResult>> { if self.starter.is_none() { - let (lib, path) = self.source.load().add_error(&mut self.report)?; + let Some((lib, path)) = self.source.load().add_error(&mut self.report)? else { + tracing::warn!( + "Plugin `{}` will not be loaded as plugin loading is disabled", + self.name + ); + return Ok(None); + }; let starter = DynamicPluginStarter::new(lib, path).add_error(&mut self.report)?; tracing::debug!("Plugin {} loaded from {}", self.name, starter.path()); self.starter = Some(starter); } else { tracing::warn!("Plugin `{}` already loaded", self.name); } - Ok(self) + Ok(Some(self)) } fn loaded(&self) -> Option<&dyn LoadedPlugin> { if self.starter.is_some() { diff --git a/plugins/zenoh-plugin-trait/src/manager/static_plugin.rs b/plugins/zenoh-plugin-trait/src/manager/static_plugin.rs index 9a67d8ce16..2354a8f926 100644 --- a/plugins/zenoh-plugin-trait/src/manager/static_plugin.rs +++ b/plugins/zenoh-plugin-trait/src/manager/static_plugin.rs @@ -84,8 +84,8 @@ where fn as_status(&self) -> &dyn PluginStatus { self } - fn load(&mut self) -> ZResult<&mut dyn LoadedPlugin> { - Ok(self) + fn load(&mut self) -> ZResult>> { + Ok(Some(self)) } fn loaded(&self) -> Option<&dyn LoadedPlugin> { Some(self) diff --git a/zenoh/src/net/runtime/adminspace.rs b/zenoh/src/net/runtime/adminspace.rs index d3e96b650f..6ee06f10fd 100644 --- a/zenoh/src/net/runtime/adminspace.rs +++ b/zenoh/src/net/runtime/adminspace.rs @@ -131,7 +131,16 @@ impl AdminSpace { ); loaded } else { - declared.load()? + match declared.load()? { + Some(loaded) => loaded, + None => { + tracing::warn!( + "Plugin `{}` will not be loaded as plugin loading is disabled", + config.name + ); + return Ok(()); + } + } }; if let Some(started) = loaded.started_mut() {