Preventing a Fn from being invoked again while it's already running

Question

I am using inputbot to write a program that provides some global macros for my computer. For example, when I press the h key, it should execute the macro typing

Hello World

into the current application. I tried to implement it like this:

extern crate inputbot;

fn main() {
    let mut callback = || {
        inputbot::KeySequence("Hello World").send();
    };

    inputbot::KeybdKey::HKey.bind(callback);

    inputbot::handle_input_events();
}

However, when I pressed the h key what I actually got was:

hHHHHHHHHHHHHHHHHHHHHHHHHHEHEHhEhEEHHhEhEhEHhEHHEHHEEHhEHlhEHEHHEHLEHLHeeleleelelelllelelleelehlhehlleeheehelheelleeleelhllllllellelolelellelleoleloloelellololol olollollelllolllol lloo ol o oo l lo lolooloooloo loo LOWOLO O L OLW WOWO L WLLOLOW L O O O O o WOWW low o oOow WWW WOW wowooWWWO oOWRWOoor W RoW oOWorororWRRWLR rLROwoRWLWOworo WorrrRWl ow o WRLR OLw o OWLDol rollWWLDWowDLlroWWo r oWDWOL dorRrwrolrdrrorlrLWDRdodRLowdllrllolrdlrddolrdlrldowldorowlrdlrorloLDLWDLoddlrddlrdldldldrrdordldrlrddrodlrrldoldlrlddldlrdlldlrdlddrlddldddlddlddd

The macro was triggering itself each time it sent the h key event.

How can I prevent a Fn from being invoked again while another instance of it is still running? This is the main functionality of a small application, so there's nothing else to really worry about compatibility with.

---

My naive attempt to fix this was to add a mut running variable in main, which callback would set to true while it was running, or immediately return if it was already true:

extern crate inputbot;

use std::time::Duration;
use std::thread::sleep;

fn main() {
    let mut running = false;
    let mut callback = || {
        if running { return };
        running = true;

        inputbot::KeySequence("Hello World").send();

        // wait to make sure keyboard events are done. 
        sleep(Duration::from_millis(125));

        running = false;
    };

    inputbot::KeybdKey::HKey.bind(callback);
    inputbot::handle_input_events();
}

However, this doesn't compile:

error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnMut`

After some reading, my understanding is now that a Fn closure (required by inputbot's .bind() methods) can't own any mutable data, like a captured mut variable.

Maybe it's possible to wrap the variable in some kind of non-mut value? Perhaps some kind-of lock, to make the potential concurrency safe, like this pseudocde?

fn main() {
    let mut running = false;
    let lockedRunning = example::Lock(&running);
    let mut callback = || {
        {
            let mut running = lockedRunning.acquire();
            if running { return };
            running = true;
        }

        inputbot::KeySequence("Hello World").send();

        // wait to make sure keyboard events are done. 
        sleep(Duration::from_millis(125));

        {
            let mut running = lockedRunning.acquire();
            running = false;
        }
    };
}

Answer 1

What you want here is that the function is mutually exclusive to itself.

Rust allows you to do this with the Mutex struct. It allows you to hold a lock that when acquired stops anyone else from taking it until you release it.

Specifically the functionality you want is the try_lock method which would allow you to check if the lock has already been acquired and would allow you to handle that case.

let lock = mutex.try_lock();

match lock {
    Ok(_) => {
       // We are the sole owners here
    }
    Err(TryLockError::WouldBlock) => return,
    Err(TryLockError::Poisoned(_)) => {
        println!("The mutex is poisoned");
        return
    }
}

Answer 2

Using an atomic value is a bit simpler than a Mutex as you don't need to worry about failure cases and it can easily be made into a static variable without using lazy-static:

use std::sync::atomic::{AtomicBool, Ordering};

fn main() {
    let is_being_called = AtomicBool::new(false);

    bind(move || {
        if !is_being_called.compare_and_swap(false, true, Ordering::SeqCst) {
            print!("I'm doing work");
            is_being_called.store(false, Ordering::SeqCst);
        }
    });
}

I have a hunch that this is also more efficient than using a Mutex as no heap allocations need to be made, but I didn't benchmark it.

If you are in a single-threaded context and your callback is somehow (accidentally?) recursive (which closures cannot be) you can also use a Cell:

use std::cell::Cell;

fn main() {
    let is_being_called = Cell::new(false);

    bind(move || {
        if !is_being_called.get() {
            is_being_called.set(true);
            print!("doing work");
            is_being_called.set(false);
        }
    })
}

If you happen to have a FnMut closure, you don't even need the Cell and can just use a boolean:

fn main() {
    let mut is_being_called = false;

    bind(move || {
        if !is_being_called {
            is_being_called = true;
            print!("doing work");
            is_being_called = false;
        }
    })
}