mirror of
https://github.com/kata-containers/kata-containers.git
synced 2025-04-29 12:14:48 +00:00
logging: Use guard to make threaded logging safe
Return a guard variable from `create_logger()` which the caller can implicitly drop to guarantee that all threads started by the async log drain are stopped. This fixes a long-standing bug [1] whereby the agent could panic with the following error, generated by the `slog` logging crate: ``` slog::Fuse Drain: Custom { kind: Other, error: "serde serialization error: Bad file descriptor (os error 9)" } ``` [1] - See https://github.com/kata-containers/kata-containers/issues/171. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
This commit is contained in:
parent
38f0d8d3ce
commit
2a648fa760
@ -21,7 +21,12 @@ const LOG_LEVELS: &[(&str, slog::Level)] = &[
|
||||
];
|
||||
|
||||
// XXX: 'writer' param used to make testing possible.
|
||||
pub fn create_logger<W>(name: &str, source: &str, level: slog::Level, writer: W) -> slog::Logger
|
||||
pub fn create_logger<W>(
|
||||
name: &str,
|
||||
source: &str,
|
||||
level: slog::Level,
|
||||
writer: W,
|
||||
) -> (slog::Logger, slog_async::AsyncGuard)
|
||||
where
|
||||
W: Write + Send + Sync + 'static,
|
||||
{
|
||||
@ -37,17 +42,21 @@ where
|
||||
let filter_drain = RuntimeLevelFilter::new(unique_drain, level).fuse();
|
||||
|
||||
// Ensure the logger is thread-safe
|
||||
let async_drain = slog_async::Async::new(filter_drain).build().fuse();
|
||||
let (async_drain, guard) = slog_async::Async::new(filter_drain)
|
||||
.thread_name("slog-async-logger".into())
|
||||
.build_with_guard();
|
||||
|
||||
// Add some "standard" fields
|
||||
slog::Logger::root(
|
||||
let logger = slog::Logger::root(
|
||||
async_drain.fuse(),
|
||||
o!("version" => env!("CARGO_PKG_VERSION"),
|
||||
"subsystem" => "root",
|
||||
"pid" => process::id().to_string(),
|
||||
"name" => name.to_string(),
|
||||
"source" => source.to_string()),
|
||||
)
|
||||
);
|
||||
|
||||
(logger, guard)
|
||||
}
|
||||
|
||||
pub fn get_log_levels() -> Vec<&'static str> {
|
||||
|
@ -168,7 +168,8 @@ fn main() -> std::result::Result<(), Box<dyn std::error::Error>> {
|
||||
// Init a temporary logger used by init agent as init process
|
||||
// since before do the base mount, it wouldn't access "/proc/cmdline"
|
||||
// to get the customzied debug level.
|
||||
let logger = logging::create_logger(NAME, "agent", slog::Level::Debug, writer);
|
||||
let (logger, logger_async_guard) =
|
||||
logging::create_logger(NAME, "agent", slog::Level::Debug, writer);
|
||||
|
||||
// Must mount proc fs before parsing kernel command line
|
||||
general_mount(&logger).map_err(|e| {
|
||||
@ -180,6 +181,7 @@ fn main() -> std::result::Result<(), Box<dyn std::error::Error>> {
|
||||
config.parse_cmdline(KERNEL_CMDLINE_FILE)?;
|
||||
|
||||
init_agent_as_init(&logger, config.unified_cgroup_hierarchy)?;
|
||||
drop(logger_async_guard);
|
||||
} else {
|
||||
// once parsed cmdline and set the config, release the write lock
|
||||
// as soon as possible in case other thread would get read lock on
|
||||
@ -222,7 +224,8 @@ fn main() -> std::result::Result<(), Box<dyn std::error::Error>> {
|
||||
let writer = unsafe { File::from_raw_fd(wfd) };
|
||||
|
||||
// Recreate a logger with the log level get from "/proc/cmdline".
|
||||
let logger = logging::create_logger(NAME, "agent", config.log_level, writer);
|
||||
let (logger, _logger_async_guard) =
|
||||
logging::create_logger(NAME, "agent", config.log_level, writer);
|
||||
|
||||
announce(&logger, &config);
|
||||
|
||||
|
@ -180,7 +180,7 @@ fn real_main() -> Result<()> {
|
||||
|
||||
// Setup logger
|
||||
let writer = io::stdout();
|
||||
let logger = logging::create_logger(name, name, log_level, writer);
|
||||
let (logger, _logger_guard) = logging::create_logger(name, name, log_level, writer);
|
||||
|
||||
announce(&logger, version);
|
||||
|
||||
|
@ -142,7 +142,7 @@ fn connect(name: &str, global_args: clap::ArgMatches) -> Result<()> {
|
||||
let log_level = logging::level_name_to_slog_level(log_level_name).map_err(|e| anyhow!(e))?;
|
||||
|
||||
let writer = io::stdout();
|
||||
let logger = logging::create_logger(name, crate_name!(), log_level, writer);
|
||||
let (logger, _guard) = logging::create_logger(name, crate_name!(), log_level, writer);
|
||||
|
||||
let timeout_nano: i64 = match args.value_of("timeout") {
|
||||
Some(t) => utils::human_time_to_ns(t).map_err(|e| e)?,
|
||||
|
Loading…
Reference in New Issue
Block a user