Storing boxed closures referencing self

help
#1

This has literally been keeping me up all night. I've created a basic Playground example of what I'm trying to do:

use std::collections::VecDeque;

struct CPU<'a> {
    value: u8,
    operations: VecDeque<Box<dyn FnMut() + 'a>>
}

impl<'me> CPU<'me> {
    pub fn get_add_two<'a>(&'a mut self) -> Box<dyn FnMut() + 'a> {
        Box::new(move || {
            self.value += 2;
        })
    }
    
    pub fn get_multiply_by_two<'a>(&'a mut self) -> Box<dyn FnMut() + 'a> {
        Box::new(move || {
            self.value = self.value * 2;
        })
    }
    
    pub fn queue(&mut self) {
        let add_two = self.get_add_two();
        let multiply_by_two = self.get_multiply_by_two();
        self.operations.push_back(add_two);
        self.operations.push_back(multiply_by_two);
    }
    
    pub fn execute(&mut self) {
        if let Some(operation) = self.operations.pop_front() {
            operation();
        }
    }
}

(Playground)

So basically: I have a CPU struct that keeps a VecDeque of boxed closures that get popped and run when the user calls execute on the CPU instance. However, it won't compile.
I get a series of error messages relating to the lifetime of the boxed closure, and it seems to boil down to:

expected `Box<(dyn FnMut() + 'me)>` found `Box<dyn FnMut()>`

I really don't understand why. Shouldn't get_add_two be returning a Box<dyn FnMut() + 'me>?

#2

Each of your closures captures a mutable borrow of the CPU. You can never have multiple mutable references to the same value, so you can never have more than one such closure. Separately, it is also difficult (impossible without the help of unsafe code, and tricky with) to make a self-referential structure, which is what you would have after the push()es.

However, it is easy to rewrite this program to not need either, by passing the CPU into the closures when they're executed, not when they're created:

use std::collections::VecDeque;

struct CPU {
    value: u8,
    operations: VecDeque<Box<dyn FnOnce(&mut CPU)>>
}

impl CPU {
    pub fn get_add_two(&self) -> Box<dyn FnOnce(&mut CPU)> {
        Box::new(move |this| {
            this.value += 2;
        })
    }
    
    pub fn get_multiply_by_two(&self) -> Box<dyn FnOnce(&mut CPU)> {
        Box::new(move |this| {
            this.value = this.value * 2;
        })
    }
    
    pub fn queue(&mut self) {
        let add_two = self.get_add_two();
        let multiply_by_two = self.get_multiply_by_two();
        self.operations.push_back(add_two);
        self.operations.push_back(multiply_by_two);
    }
    
    pub fn execute(&mut self) {
        if let Some(operation) = self.operations.pop_front() {
            operation(self);
        }
    }
}

As a general rule, think twice any time you write a struct with a lifetime parameter. It's much less often the right answer than it might seem.

7 Likes
#3

I don't think this API is possible at all because if you write

fn get_add_two<'a>(&'a mut self) -> Box<dyn FnMut() + 'a>

you keep the borrow to &mut self alive until the resulting Box is dropped
and since you only can have one &mut borrow at the time this

let add_two = self.get_add_two();
let multiply_by_two = self.get_multiply_by_two();

can not work

#4

Don't know why I didn't see that.I think I got so focussed on getting the self capturing by the closures to work that I didn't think twice about he fact that those references are captured for the entire lifetime of the closure. Thanks for making me see the error of my ways.