Use static LOGGER not static mut LOGGER (#215)

crates/ark/src/logger.rs

@@ -16,7 +16,6 @@ use chrono::DateTime;
 use chrono::Utc;
 use lazy_static::lazy_static;
 use regex::Regex;
-use stdext::unwrap;
 
 lazy_static! {
     static ref RE_ARK_BACKTRACE: Regex = Regex::new("^\\s*\\d+:\\s*[<]?ark::").unwrap();
@@ -76,51 +75,63 @@ fn is_internal(record: &log::Record) -> bool {
     }
 }
 
-struct Logger {
+static ONCE: Once = Once::new();
+static LOGGER: Logger = Logger::new();
+
+struct LoggerInner {
     /// The log level (set with the RUST_LOG environment variable)
     level: log::Level,
 
+    /// The file we log to.
+    /// None if no log file has been specified (we log to stdout in this case).
+    file: Option<File>,
+}
+
+struct Logger {
     /// A mutex to ensure that only one thread is writing to the log file at a
-    /// time; None if no log file has been specified (we log to stdout in this
-    /// case)
-    mutex: Option<Mutex<File>>,
+    /// time. Set to `None` at compile time, set to a real result during `initialize()`.
+    /// Also required for interior mutability while still being able to have a static
+    /// reference to supply to `log::set_logger()`.
+    inner: Mutex<Option<LoggerInner>>,
 }
 
 impl Logger {
-    fn initialize(&mut self, file: Option<&str>) {
-        self.mutex = None;
-
-        if let Some(file) = file {
-            let file = std::fs::OpenOptions::new()
-                .write(true)
-                .append(true)
-                .create(true)
-                .open(file);
-
-            match file {
-                Ok(file) => self.mutex = Some(Mutex::new(file)),
-                Err(error) => eprintln!("Error initializing log: {error:?}"),
-            }
-        }
+    const fn new() -> Self {
+        let inner = Mutex::new(None);
+        Self { inner }
+    }
+
+    fn initialize(&self, level: log::Level, file: Option<File>) {
+        let mut inner = self.inner.lock().unwrap();
+        *inner = Some(LoggerInner { level, file });
+    }
+
+    fn enabled(level: log::Level, metadata: &log::Metadata) -> bool {
+        metadata.level() as i32 <= level as i32
     }
 }
 
 impl log::Log for Logger {
     fn enabled(&self, metadata: &log::Metadata) -> bool {
-        metadata.level() as i32 <= self.level as i32
+        let guard = self.inner.lock().unwrap();
+        let inner = guard.as_ref().unwrap();
+        Logger::enabled(inner.level, metadata)
     }
 
     fn log(&self, record: &log::Record) {
-        if !self.enabled(record.metadata()) {
-            return;
-        }
-
         if !is_internal(record) && record.level() > log::Level::Warn {
             // To avoid a noisy output channel, we don't log information
             // from foreign crates unless they are warnings or errors
             return;
         }
 
+        let mut guard = self.inner.lock().unwrap();
+        let inner = guard.as_mut().unwrap();
+
+        if !Logger::enabled(inner.level, record.metadata()) {
+            return;
+        }
+
         // Generate timestamp.
         let now: DateTime<Utc> = SystemTime::now().into();
         let timestamp = now.to_rfc3339_opts(chrono::SecondsFormat::Nanos, true);
@@ -145,13 +156,11 @@ impl log::Log for Logger {
         // Generate message to log.
         let message = format!("{prefix}: {message}");
 
-        if let Some(mutex) = self.mutex.as_ref() {
+        if let Some(file) = inner.file.as_mut() {
             // Write to log file if one is specified.
-            if let Ok(mut file) = mutex.lock() {
-                let status = writeln!(file, "{}", message);
-                if let Err(error) = status {
-                    eprintln!("Error writing to log file: {error:?}");
-                }
+            let status = writeln!(file, "{}", message);
+            if let Err(error) = status {
+                eprintln!("Error writing to log file: {error:?}");
             }
         } else {
             // If no log file is specified, write to stdout.
@@ -164,7 +173,10 @@ impl log::Log for Logger {
     }
 
     fn flush(&self) {
-        if let Ok(mut file) = self.mutex.as_ref().unwrap().lock() {
+        let mut guard = self.inner.lock().unwrap();
+        let inner = guard.as_mut().unwrap();
+
+        if let Some(file) = inner.file.as_mut() {
             file.flush().unwrap();
         }
     }
@@ -174,27 +186,37 @@ pub fn initialize(file: Option<&str>) {
     ONCE.call_once(|| {
         // Initialize the log level, using RUST_LOG.
         let level_envvar = std::env::var("RUST_LOG").unwrap_or("info".into());
-        let level = unwrap!(
-            log::Level::from_str(level_envvar.as_str()),
-            Err(error) => {
-                eprintln!("Error parsing RUST_LOG: {error:?}");
-                return;
-            }
-        );
+
+        let level = match log::Level::from_str(level_envvar.as_str()) {
+            Ok(level) => level,
+            Err(err) => {
+                eprintln!("Error parsing RUST_LOG, defaulting to `info`: {err:?}");
+                log::Level::Info
+            },
+        };
 
         log::set_max_level(level.to_level_filter());
 
-        // Set up the logger
-        unsafe {
-            LOGGER.level = level;
-            LOGGER.initialize(file);
-            log::set_logger(&LOGGER).unwrap();
+        let file = match file {
+            None => None,
+            Some(file) => {
+                let file = std::fs::OpenOptions::new()
+                    .write(true)
+                    .append(true)
+                    .create(true)
+                    .open(file);
+
+                match file {
+                    Ok(file) => Some(file),
+                    Err(error) => {
+                        eprintln!("Error initializing log: {error:?}");
+                        None
+                    },
+                }
+            },
         };
+
+        LOGGER.initialize(level, file);
+        log::set_logger(&LOGGER).unwrap();
     });
 }
-
-static ONCE: Once = Once::new();
-static mut LOGGER: Logger = Logger {
-    mutex: None,
-    level: log::Level::Info,
-};