Need help hacking a Debug impl to read from global variable

help
1

I have some tests that check the output of a heavily nested type, this is currently done by using the pretty debug output ("{:?}") and the expected output is stored in "golden" files. I'm using a crate I built for this: goldie.

The type is an AST and I have a Span struct which tends to be a leaf node of some things in the AST. It only contains positions in the text not the actual source that the AST was parsed from. I wanted to change this to output the location in the source instead of the positions and instead of writing an entire pretty printer I wanted to continue to use the Debug output and temporarily override the debug implementation of the Span (I know this is terrible hack, but I figure it's just for tests so bear with me).

For example:

pub struct Span {
    pub m: usize,
    pub n: usize,
}

#[cfg(not(test))]
impl fmt::Debug for Span {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        fmt::Debug::fmt(&(self.m..self.n), f)
    }
}

#[cfg(test)]
impl fmt::Debug for Span {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match { /* try get source */ } {
            Some(source) => fmt::Debug::fmt(&source[self.m..self.n], f),
            None => fmt::Debug::fmt(&(self.m..self.n), f),
        }
    }
}

I can do this using a global variable but then it doesn't work when tests are run in parallel. I figured I need to do something like the following

  • Lock some global or thread local variable and get some sort of RAII guard
  • Set the variable with the current source we are parsing.
  • Parse the AST, format and verify the Debug output which will read the variable
  • Drop the guard

The test would have to be written like this

#[test]
fn example() {
    let source = "...";
    let _guard = crate::span::set_source(source);
    let ast = parse_ast(source);
    goldie::assert_debug!(&ast);
}

I'm not sure what type of synchronization type this is? I tried implementing it using std::sync::Mutex and parking_lot::ReentrantMutex but I realized I need something that has separate conditions for reading and writing. Any ideas what to use here?

2

You mean like RwLock? Anyway, I don't understand why you would need "separate conditions for reading and writing". Can you elaborate?

Thread-locals are local to the thread. You don't need to lock them in multi-threaded code, because only one thread can access them.

1 Like
3

I'd personally just run tests sequentially (if all tests need access to that global variable) or force those tests that do to be executed sequentially with a mutex in your test module. The former can be achieved with cargo test -- --test-threads=1 and the latter looks something like:

#[cfg(test)]
mod tests {
    use std::sync::{Arc, Mutex};
    
    use lazy_static::lazy_static;
    
    lazy_static! { 
        static ref MUTEX: Arc<Mutex<()>> = Arc::new(Mutex::new(()));
    }
    
    #[test]
    fn test1() {
        let _lock = MUTEX.lock().unwrap();
        
        assert!(true);
    }
    
    #[test]
    fn test2() {
        let _lock = MUTEX.lock().unwrap();
        
        assert!(true);
    }
}

This would force test2 to wait till test1 is done before it can execute.

4

I think I figured out your problem and it is not read/write related. You simply deadlock, because you don't drop the mutex between writing and reading.

You need to drop the _guard before parse_ast (which I assume is where you reaquire the mutex). Look closely at the difference between test1 and test2 in this example:

use std::sync::{Arc, Mutex};

use lazy_static::lazy_static;
    
lazy_static! { 
    static ref GLOB: Arc<Mutex<String>> = Arc::new(Mutex::new("".to_owned()));
}

fn use_glob() -> String {
    let s = GLOB.lock().unwrap();
    println!("{s}");
    s.clone()
}

#[cfg(test)]
mod tests {
    use super::{GLOB, use_glob};
    
    // ! THIS WILL DEADLOCK
    fn test1() {
        let mut s = GLOB.lock().unwrap();
        *s = "HELLO".to_owned();
        
        let res = use_glob(); // will never aquire lock, so runs forever
        
        assert_eq!(res, "HELLO");
    }
    
    #[test]
    fn test2() {
        {
            let mut s = GLOB.lock().unwrap();
            *s = "HELLO".to_owned();
        } // lock dropped here, so we can aquire it in `use_glob`
        
        let res = use_glob();
        
        assert_eq!(res, "HELLO");
    }
}

Playground.

You need to let go of the mutex after you've updated the value it is guarding. Otherwise it can never be reacquired.

5

Using a single global Mutex<Option<String>> is not enough because ideally you need to hold the lock across both the part where the value is set and later when it is read in the Debug implementation.

For example this is what I first tried. It only works if you run with --test-threads=1.

use std::sync::Mutex;
use std::fmt;
use once_cell::sync::Lazy;

static SOURCE: Lazy<Mutex<Option<String>>> = Lazy::new(|| Mutex::new(None));

impl fmt::Debug for Span {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match SOURCE.lock().unwrap().as_deref() {
            Some(source) => fmt::Debug::fmt(&source[self.m..self.n], f),
            None => fmt::Debug::fmt(&(self.m..self.n), f),
        }
    }
}

#[test]
fn example() {
    let source = "...";
    SOURCE.lock().unwrap().replace(source.to_owned());
    let ast = parse_ast(source);
    goldie::assert_debug!(&ast);
}

Anyway I actually implemented this using thread_local! and it works fine.

use std::cell::RefCell;
use std::fmt;

impl fmt::Debug for Span {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        SOURCE.with(|s| match s.borrow().as_deref() {
            Some(source) => fmt::Debug::fmt(&source[self.m..self.n, f),
            None => fmt::Debug::fmt(&(self.m..self.n), f),
        })
    }
}

thread_local! {
    static SOURCE: RefCell<Option<String>> = RefCell::new(None);
}


#[test]
fn example() {
    let source = "...";
    SOURCE.with(|s| s.borrow_mut().replace(source.to_owned()));
    let ast = parse_ast(source);
    goldie::assert_debug!(&ast);
}
6

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.