Immutatable References to Mutex Value / "Global" Atomic Boolean

Hello Rust Community,
this is my first thread and my first post, since I started learning Rust some days ago.
I wrote a small programm based on the Command Line Programm-Tutorial, which basically aims to write a heavily simplified Rust version of “grep”.

After reading the tutorial to a certain point, I decided to extend the programm in order for it to work recursively through directories. This of course creates the need to gracefully abort parsing when CTRL-C is hit, for instance when working on really large folders.

I implemented a read_directory function that basically calls fs::read_dir and steps through the result inserting the content of each file into a HashMap with the file path as a key. The function recursivley calls itself if a listed file is actually a directory.

The idea is, that I break the for loop and the recursion of the read_directory function if a “global” boolean named “shutdown” is true.
Shutdown needs to be true if CTRL-C is hit.

In order to achive this, I added a shutdown handler via ctrlc:
ctrlc::set_handler(shutdown_hook).expect("Failed to set shutdown hook");

Originally I decided to create a mutable boolean, move it to the shutdown_hook closure and pass a immutatable reference to all further functions like read_directory.

Since the set_handler function only accepts Fn, I could not move the shutdown boolean to the closure.

So I decided to create a Mutex<bool>, which works fine with the shutdown_hook clouse, like this:

let shutdown = Mutex::new(false);

let shutdown_hook = move || {
    let mut s = shutdown.lock().expect("Failed to aquire lock");
    if !*s {
        *s = true;
        println!("Aborting...");
    }
};  
ctrlc::set_handler(shutdown_hook).expect("Failed to set shutdown hook");

The problem is that since I moved the Mutex to the closure, I now can not pass a reference to the Mutex to read_directory. I’m thus forced to create atomic references via Arc.
This leads to ugly code like this though (taken from read_directory):

fn read_directory<'a>(map : &'a mut HashMap<String, String>, path : &str, shutdown : Arc<Mutex<bool>>) -> Result<&'a mut HashMap<String, String>, Box<dyn Error>> {
    // ...
    let shutdown_ref2 = Arc::clone(&shutdown);
    let shutdown_bool = shutdown.lock().unwrap();
    if *shutdown_bool {
        break;
    }
    std::mem::drop(shutdown_bool);
    // ...
    if Path::new(&p).is_dir() {
        read_directory(map, &p, shutdown_ref2).expect(&format!("Failed to read directory '{}'", &p));
    } else {
        // ...
    }
}

So now, I have only have one major questions:
How can I improve my Mutex/Arc logic? I basically only want the shutdown_hook to manipulate the wrapped boolean, so I don’t even need all the other parts of the application to have write-access via the Arc to the Mutex.

I’m afraid I completely missunderstood some concepts.

Can someone maybe help me, with some advice.
I’m also willing to post the whole code of the application.

Hi, if your program is single threaded you can go with an Rc<RefCell<bool>> if it’s multithreaded an Arc<AtomicBool> should be lighter weight than Arc<Mutex<bool>>.

1 Like

That doesn’t follow: you can move anything into a Fn closure, as long as it isn’t moved when the closure is called. But I agree that moving a bool into the closure has the same limitation moving a Mutex<bool> has: you can’t take a reference to it once it’s a member of the closure.

However, you can move a reference to a Mutex<bool> into the closure, as long as it’s 'static (set_handler has a 'static bound on F). So one solution would be (untested):

static SHUTDOWN: Mutex<bool> = Mutex::new(false);
ctrlc::set_handler(|| {  // note lack of `move`
    let mut s = SHUTDOWN.lock().expect("Failed to aquire lock");
    if !*s {
        *s = true;
        println!("Aborting...");
    }
};

Consider using AtomicBool instead of Mutex<bool>, as @leudz also mentioned.

Making the flag static isn’t quite as flexible as putting it in an Arc, but for the use case you describe, it’s probably correct; note that setting a signal handler effectively leaks the Arc, since you can’t call set_handler twice. You can also declare static SHUTDOWN at the module level and use it in read_directory without passing in a reference, although maybe you prefer to have it passed in explicitly.

Okay, thanks for your answers so far. They really helped me to correct some of my misunderstandings.

@trentj could you please elaborate further on this:

I didn’t quiet understand what you mean here.

The handler closure will never be dropped – that’s why it has to have the 'static lifetime. But an Arc has to be dropped to free up its allocation. So moving an Arc into a closure and passing it to set_handler leaks the memory associated with the Arc.

The second sentence is about code organization. In your original example, you pass the Arc down through read_directory, which you can also do with plain references (AtomicBool version):

fn read_directory<'a>(
    map: &'a mut HashMap<String, String>,
    path: &str,
    shutdown: &AtomicBool,
) -> Result<&'a mut HashMap<String, String>, Box<dyn Error>> {
    // check using local:
    if shutdown.load(Ordering::SeqCst) {
        break;
    }
    // recursive call:
    read_directory(..., shutdown)
}

fn main() {
    static SHUTDOWN: AtomicBool = AtomicBool::new(false);
    ctrlc::set_handler(|| {
        if !SHUTDOWN.swap(true, Ordering::SeqCst) {
            println!("Aborting...");
        }
    });
    
    let mut map = HashMap::new();
    let _ = read_directory(&mut map, "/", &SHUTDOWN);
}

But since SHUTDOWN is static anyway, you could lift it out of main and use it directly in read_directory:

static SHUTDOWN: AtomicBool = AtomicBool::new(false);

fn read_directory<'a>(
    map: &'a mut HashMap<String, String>,
    path: &str,
) -> Result<&'a mut HashMap<String, String>, Box<dyn Error>> {
    // check using global:
    if SHUTDOWN.load(Ordering::SeqCst) {
        break;
    }
    // recursive call:
    read_directory(...)
}

fn main() {
    ctrlc::set_handler(|| {
        if !SHUTDOWN.swap(true, Ordering::SeqCst) {
            println!("Aborting...");
        }
    });
    
    let mut map = HashMap::new();
    let _ = read_directory(&mut map, "/");
}

The downsides being:

  • read_directory is now coupled to SHUTDOWN, so you can’t make an “uninterruptible” read_directory call by passing a reference to a different AtomicBool
  • The shutdown flag no longer appears in the call signature of read_directory, so the dependency is hidden.

On the other hand, using a global static is simpler and more precisely expresses the meaning that all read_directory calls should stop on Ctrl-C.

Okay, thanks, this makes sense.
But how could I avoid the leak? Technically I could leave it like this, since the kernal will clean up the leak after the termination of the program. Still, I’m interested in what a clean approach would look like.

You can avoid leaking a heap allocation by not allocating in the first place – that’s the static solution. But you can’t avoid that piece of global state, which is why there’s no real point in using Arc over static in this particular case.

In some situations you might use Arc instead of a static, even for state that is effectively global, because a static can’t be cleaned up. But since a signal handler is never cleaned up anyway, that doesn’t apply in this case.

Okay, thanks for all your responses.
I didn’t know that static variables can’t (or don’t need to be cleaned up), but it makes sense, since they are effectively in no scope.