Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

env_logger cannot be used as root logger because it is not unwind safe #264

Open
e00E opened this issue Jun 8, 2020 · 5 comments
Open
Labels
C-bug Category: This is a bug. P-low Priority: Low slog-envlogger crate: slog-envlogger

Comments

@e00E
Copy link

e00E commented Jun 8, 2020

use slog::Drain;

fn main() {
    let drain = slog::Discard;
    let drain = slog_envlogger::LogBuilder::new(drain).build();
    // Does not help:
    // let drain = drain.fuse();
    let logger = slog::Logger::root(drain, slog::o!());
}

error[E0277]: the type std::cell::UnsafeCell<std::option::Option<std::boxed::Box<std::cell::RefCell<regex::exec::ProgramCacheInner>>>> may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
--> src/main.rs:8:18
|
8 | let logger = slog::Logger::root(drain, slog::o!());
| ^^^^^^^^^^^^^^^^^^ std::cell::UnsafeCell<std::option::Option<std::boxed::Box<std::cell::RefCell<regex::exec::ProgramCacheInner>>>> may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
|
::: /.cargo/registry/src/github.com-1ecc6299db9ec823/slog-2.5.2/src/lib.rs:1120:38
|
1120 | T: SendSyncRefUnwindSafeKV + 'static,
| ------- required by this bound in slog::Logger::<D>::root
|
= help: within slog_envlogger::EnvLogger<slog::Discard>, the trait std::panic::RefUnwindSafe is not implemented for std::cell::UnsafeCell<std::option::Option<std::boxed::Box<std::cell::RefCell<regex::exec::ProgramCacheInner>>>>
= note: required because it appears within the type thread_local::cached::CachedThreadLocal<std::cell::RefCell<regex::exec::ProgramCacheInner>>
= note: required because it appears within the type regex::cache::imp::Cached<std::cell::RefCell<regex::exec::ProgramCacheInner>>
= note: required because it appears within the type regex::exec::Exec
= note: required because it appears within the type regex::re_unicode::Regex
= note: required because it appears within the type slog_envlogger::filter::Filter
= note: required because it appears within the type std::option::Option<slog_envlogger::filter::Filter>
= note: required because it appears within the type slog_envlogger::EnvLogger<slog::Discard>
= note: required because of the requirements on the impl of slog::SendSyncRefUnwindSafeDrain for slog_envlogger::EnvLogger<slog::Discard>

I find this surprising. In one of the examples https://github.com/slog-rs/envlogger/blob/v2.2.0/examples/proper.rs the workaround seems to be that env_logger is wrapped in the async logger.

@dpc
Copy link
Collaborator

dpc commented Jun 8, 2020

You probably want to use it behind an async anyway (for perf.), but yeah ... it probably shouldn't be much work to fix this. Please fill a PR! :)

@e00E
Copy link
Author

e00E commented Jun 11, 2020

I'll look into making a PR to fix this.

This actually came up because I wanted the logging chain to be slog_envlogger -> slog_async instead of what is probably more common slog_async -> slog_envlogger.
The reason for that is that I don't want messages that don't match the envlogger filter to end up on the async channel at all so that a for example a library cannot fill up the channel with trace logs (leading to dropped logs) when they are going to be filtered out anyway.
Checking the log message against the filter probably has roughly similar performance as putting them on the channel in the first place.

@e00E
Copy link
Author

e00E commented Jun 11, 2020

Could you elaborate why slog requires types to be unwind safe?
Edit: found #111

@e00E
Copy link
Author

e00E commented Jun 11, 2020

Relevant regex issue rust-lang/regex#576 .

@Techcable
Copy link
Member

Hmm, you could use AssertUnwindSafe as a quick-and-dirty fix.

I think in general the problem the compiler is worried about is that panicking may cause the logger to appear in an inconsistent state (because of interior mutability). Not sure if that's specifically a problem in slog_envlogger

@Techcable Techcable added P-low Priority: Low slog-envlogger crate: slog-envlogger C-bug Category: This is a bug. labels Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. P-low Priority: Low slog-envlogger crate: slog-envlogger
Projects
None yet
Development

No branches or pull requests

3 participants