event_monitor: Replace unsound static mut MONITOR

See discussion in https://github.com/rust-lang/rust/issues/53639.
This came up during an internal review.

Signed-off-by: Christian Blichmann <cblichmann@google.com>
This commit is contained in:
Christian Blichmann 2023-09-14 11:19:12 +02:00 committed by Rob Bradford
parent f557aadb67
commit 88f3537b47
3 changed files with 13 additions and 13 deletions

1
Cargo.lock generated
View File

@ -624,6 +624,7 @@ version = "0.1.0"
dependencies = [ dependencies = [
"flume", "flume",
"libc", "libc",
"once_cell",
"serde", "serde",
"serde_json", "serde_json",
] ]

View File

@ -7,5 +7,6 @@ edition = "2021"
[dependencies] [dependencies]
flume = "0.10.14" flume = "0.10.14"
libc = "0.2.147" libc = "0.2.147"
once_cell = "1.18.0"
serde = { version = "1.0.168", features = ["rc", "derive"] } serde = { version = "1.0.168", features = ["rc", "derive"] }
serde_json = "1.0.96" serde_json = "1.0.96"

View File

@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0 // SPDX-License-Identifier: Apache-2.0
// //
use once_cell::sync::OnceCell;
use serde::Serialize; use serde::Serialize;
use std::borrow::Cow; use std::borrow::Cow;
use std::collections::HashMap; use std::collections::HashMap;
@ -12,7 +13,7 @@ use std::os::unix::io::AsRawFd;
use std::sync::Arc; use std::sync::Arc;
use std::time::{Duration, Instant}; use std::time::{Duration, Instant};
static mut MONITOR: Option<MonitorHandle> = None; static MONITOR: OnceCell<MonitorHandle> = OnceCell::new();
#[derive(Serialize)] #[derive(Serialize)]
struct Event<'a> { struct Event<'a> {
@ -69,8 +70,8 @@ fn set_file_nonblocking(file: &File) -> io::Result<()> {
/// This function must only be called once from the main thread before any threads /// This function must only be called once from the main thread before any threads
/// are created to avoid race conditions. /// are created to avoid race conditions.
pub fn set_monitor(file: Option<File>) -> io::Result<Monitor> { pub fn set_monitor(file: Option<File>) -> io::Result<Monitor> {
// SAFETY: there is only one caller of this function, so MONITOR is written to only once // There is only one caller of this function, so MONITOR is written to only once
assert!(unsafe { MONITOR.is_none() }); assert!(MONITOR.get().is_none());
if let Some(ref file) = file { if let Some(ref file) = file {
set_file_nonblocking(file)?; set_file_nonblocking(file)?;
@ -79,24 +80,21 @@ pub fn set_monitor(file: Option<File>) -> io::Result<Monitor> {
let (tx, rx) = flume::unbounded(); let (tx, rx) = flume::unbounded();
let monitor = Monitor::new(rx, file); let monitor = Monitor::new(rx, file);
// SAFETY: MONITOR is None. Nobody else can hold a reference to it. MONITOR.get_or_init(|| MonitorHandle {
unsafe {
MONITOR = Some(MonitorHandle {
tx, tx,
start: Instant::now(), start: Instant::now(),
}); });
};
Ok(monitor) Ok(monitor)
} }
pub fn event_log(source: &str, event: &str, properties: Option<&HashMap<Cow<str>, Cow<str>>>) { pub fn event_log(source: &str, event: &str, properties: Option<&HashMap<Cow<str>, Cow<str>>>) {
// SAFETY: `MONITOR` is always in a valid state (None or Some), because it // `MONITOR` is always in a valid state (None or Some), because it is set
// is set only once before any threads are spawned, and it's not mutated // only once before any threads are spawned, and it's not mutated
// afterwards. This function only creates immutable references to `MONITOR`. // afterwards. This function only creates immutable references to `MONITOR`.
// Because `MONITOR.tx` is `Sync`, it's safe to share `MONITOR` across // Because `MONITOR.tx` is `Sync`, it's safe to share `MONITOR` across
// threads, making this function thread-safe. // threads, making this function thread-safe.
if let Some(monitor_handle) = unsafe { MONITOR.as_ref() } { if let Some(monitor_handle) = MONITOR.get().as_ref() {
let event = Event { let event = Event {
timestamp: monitor_handle.start.elapsed(), timestamp: monitor_handle.start.elapsed(),
source, source,