Skip to content

Commit

Permalink
sound: Put AlsaTestHarness static in a LazyLock
Browse files Browse the repository at this point in the history
Instead of using unsafe {} and undefined behavior (mutable shared static
references) use LazyLock which initializes the harness on first
dereference and provides immutable access only (the harness type has
interior mutability).

Fixes: rust-vmm#785

Signed-off-by: Manos Pitsidianakis <[email protected]>
  • Loading branch information
epilys committed Dec 2, 2024
1 parent 4ea77f5 commit 8ec80cb
Showing 1 changed file with 22 additions and 84 deletions.
106 changes: 22 additions & 84 deletions vhost-device-sound/src/audio_backends/alsa/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,58 +3,31 @@

use std::{
path::PathBuf,
sync::{
atomic::{AtomicU8, Ordering},
Arc, Mutex, Once,
},
sync::{Arc, LazyLock, Mutex},
};

use tempfile::{tempdir, TempDir};

static mut TEST_HARNESS: Option<AlsaTestHarness> = None;
static INIT_ALSA_CONF: Once = Once::new();
static TEST_HARNESS: LazyLock<AlsaTestHarness> = LazyLock::new(AlsaTestHarness::new);

#[must_use]
pub fn setup_alsa_conf() -> AlsaTestHarnessRef<'static> {
INIT_ALSA_CONF.call_once(||
// SAFETY:
// This is only called once, because of.. `Once`, so it's safe to
// access the static value mutably.
unsafe {
TEST_HARNESS = Some(AlsaTestHarness::new());
});
let retval = AlsaTestHarnessRef(
// SAFETY:
// The unsafe { } block is needed because TEST_HARNESS is a mutable static. The inner
// operations are protected by atomics.
unsafe { TEST_HARNESS.as_ref().unwrap() },
);
retval.0.inc_ref();
retval
pub fn setup_alsa_conf() -> &'static AlsaTestHarness {
// Dereferencing is necessary to perform the LazyLock init fn
&TEST_HARNESS
}

/// The alsa test harness. It must only be constructed via
/// `AlsaTestHarness::new()`.
#[non_exhaustive]
/// The alsa test harness.
///
/// It sets up the `ALSA_CONFIG_PATH` variable pointing to a configuration file
/// inside a temporary directory. This way the tests won't mess with the host
/// machine's devices.
pub struct AlsaTestHarness {
pub tempdir: Arc<Mutex<Option<TempDir>>>,
pub conf_path: PathBuf,
pub ref_count: AtomicU8,
}

/// Ref counted alsa test harness ref.
#[repr(transparent)]
#[non_exhaustive]
pub struct AlsaTestHarnessRef<'a>(&'a AlsaTestHarness);

impl<'a> Drop for AlsaTestHarnessRef<'a> {
fn drop(&mut self) {
self.0.dec_ref();
}
tempdir: Arc<Mutex<Option<TempDir>>>,
conf_path: PathBuf,
}

impl AlsaTestHarness {
pub fn new() -> Self {
fn new() -> Self {
let tempdir = tempdir().unwrap();
let conf_path = tempdir.path().join("alsa.conf");

Expand All @@ -74,55 +47,20 @@ impl AlsaTestHarness {
Self {
tempdir: Arc::new(Mutex::new(Some(tempdir))),
conf_path,
ref_count: 0.into(),
}
}

#[inline]
pub fn inc_ref(&self) {
let old_val = self.ref_count.fetch_add(1, Ordering::SeqCst);
assert!(
old_val != u8::MAX,
"ref_count overflowed on 8bits when increasing by 1"
);
}

#[inline]
pub fn dec_ref(&self) {
let old_val = self.ref_count.fetch_sub(1, Ordering::SeqCst);
if old_val == 1 {
let mut lck = self.tempdir.lock().unwrap();
println!(
"INFO: unsetting ALSA_CONFIG_PATH={} in PID {} and TID {:?}",
self.conf_path.display(),
std::process::id(),
std::thread::current().id()
);
std::env::remove_var("ALSA_CONFIG_PATH");
_ = lck.take();
}
}
}

impl Drop for AlsaTestHarness {
fn drop(&mut self) {
let ref_count = self.ref_count.load(Ordering::SeqCst);
if ref_count != 0 {
println!(
"ERROR: ref_count is {ref_count} when dropping {}",
stringify!(AlsaTestHarness)
);
}
if self
.tempdir
.lock()
.map(|mut l| l.take().is_some())
.unwrap_or(false)
{
println!(
"ERROR: tempdir held a value when dropping {}",
stringify!(AlsaTestHarness)
);
}
let mut lck = self.tempdir.lock().unwrap();
println!(
"INFO: unsetting ALSA_CONFIG_PATH={} in PID {} and TID {:?}",
self.conf_path.display(),
std::process::id(),
std::thread::current().id()
);
std::env::remove_var("ALSA_CONFIG_PATH");
_ = lck.take();
}
}

0 comments on commit 8ec80cb

Please sign in to comment.