From 88f3537b4753c6ff0efb9c7ad61ccd1747b5a701 Mon Sep 17 00:00:00 2001
From: Christian Blichmann <cblichmann@google.com>
Date: Thu, 14 Sep 2023 11:19:12 +0200
Subject: [PATCH] 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>
---
 Cargo.lock               |  1 +
 event_monitor/Cargo.toml |  1 +
 event_monitor/src/lib.rs | 24 +++++++++++-------------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 317796ac7..d6fa229a3 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -624,6 +624,7 @@ version = "0.1.0"
 dependencies = [
  "flume",
  "libc",
+ "once_cell",
  "serde",
  "serde_json",
 ]
diff --git a/event_monitor/Cargo.toml b/event_monitor/Cargo.toml
index 900e10df4..a01225463 100644
--- a/event_monitor/Cargo.toml
+++ b/event_monitor/Cargo.toml
@@ -7,5 +7,6 @@ edition = "2021"
 [dependencies]
 flume = "0.10.14"
 libc = "0.2.147"
+once_cell = "1.18.0"
 serde = { version = "1.0.168", features = ["rc", "derive"] }
 serde_json = "1.0.96"
diff --git a/event_monitor/src/lib.rs b/event_monitor/src/lib.rs
index c6cf10f6a..482085c21 100644
--- a/event_monitor/src/lib.rs
+++ b/event_monitor/src/lib.rs
@@ -3,6 +3,7 @@
 // SPDX-License-Identifier: Apache-2.0
 //
 
+use once_cell::sync::OnceCell;
 use serde::Serialize;
 use std::borrow::Cow;
 use std::collections::HashMap;
@@ -12,7 +13,7 @@ use std::os::unix::io::AsRawFd;
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 
-static mut MONITOR: Option<MonitorHandle> = None;
+static MONITOR: OnceCell<MonitorHandle> = OnceCell::new();
 
 #[derive(Serialize)]
 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
 /// are created to avoid race conditions.
 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
-    assert!(unsafe { MONITOR.is_none() });
+    // There is only one caller of this function, so MONITOR is written to only once
+    assert!(MONITOR.get().is_none());
 
     if let Some(ref file) = 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 monitor = Monitor::new(rx, file);
 
-    // SAFETY: MONITOR is None. Nobody else can hold a reference to it.
-    unsafe {
-        MONITOR = Some(MonitorHandle {
-            tx,
-            start: Instant::now(),
-        });
-    };
+    MONITOR.get_or_init(|| MonitorHandle {
+        tx,
+        start: Instant::now(),
+    });
 
     Ok(monitor)
 }
 
 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
-    // is set only once before any threads are spawned, and it's not mutated
+    // `MONITOR` is always in a valid state (None or Some), because it is set
+    // only once before any threads are spawned, and it's not mutated
     // afterwards. This function only creates immutable references to `MONITOR`.
     // Because `MONITOR.tx` is `Sync`, it's safe to share `MONITOR` across
     // 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 {
             timestamp: monitor_handle.start.elapsed(),
             source,