Cannot move out of captured outer variable in an `FnMut` closure

Hello all, new to rust and encountering the error cannot move out of captured outer variable in an FnMut closure with the following program

I know that it's img that causes this and I've tried using, inside the map, let img = &mut img; however that gives a different error of cannot infer an appropriate lifetime for borrow expression due to conflicting requirements

I was hoping someone could help me solve this as I'm new to rust and can't quite figure it out.

Thanks in advance!

Hi,

As img needs to be mutably shared among threads, it needs to be protected by mutex. But we cannot share a mutex among threads, so the mutex needs to be ARCed, the same way you're handling rx.

    let cpus = num_cpus::get();
    let (tx, rx) = channel();
    let rx = Arc::new(Mutex::new(rx));
    let img_buffer: image::GrayImage = image::ImageBuffer::new(WIDTH, HEIGHT); //
    let img_arc = Arc::new(Mutex::new(img_buffer));  // 
    let thread_handles = (0..cpus)
        .map(|_| {
            let rx = rx.clone();
            let img_arc_clone = img_arc.clone();

            thread::spawn(move || loop {
                let val = rx.lock().unwrap().recv();
                let mut img = img_arc_clone.lock().unwrap(); //
                match val {
                    Err(_) => break,
                    Ok(x) => {
                        for y in 0..HEIGHT {
                            img.put_pixel(x, y, image::Luma([generate_pixel(x, y) as u8]));
                        }
                    },
                };
            })

        })
        .collect::<Vec<_>>();

    for x in 0..WIDTH {
        let _ = tx.send(x);
    }
    drop(tx);

    for h in thread_handles {
        let _ = h.join();
    }

This can mitigate the error, though i don't know if this is the best solution. https://github.com/rust-lang/rust/issues/31752 shows (as you also mentioned) that a mutable ref can be used as well, but, then i don't know how to solve the lifetime issue. Drawback of this method is that we're now using 2 mutexes.

On the other hand (CMIIW), i doubt if this correctly parallelizes the computation, as at any point, 1 column will be rendered because at any time only 1 thread will be able to read from the rx. So, if we're on 4 core machine, there will be 5 threads (4 doing computation on image and 1 main thread), out of which, at any given time, 2 will be active, 1 main and 1 renderer.

And, computation pattern looks like spmc (Single Producer Multiple Consumers). spmc - Rust

PS: This is my second month in rust. Enjoying it very much, though, it can be very hard at times :slight_smile:

As @lafolle pointed out, your code uses shared mutable state, in the form of the "img" shared variable. Rust is not a big fan of this, in no small part due to the thread-safety implications: in the presence of shared mutability how does one ensure that two threads will never be writing to the same data at the same time?

Throwing a mutex into the mix would make the compiler happy, but at a stringent performance cost: your threads would now need to synchronize and fight for the lock, which is unnecessary here because the parts of the image which each thread is accessing are independent.


The idiomatic way to do what you have in mind in Rust is the following:

  • Split your image into a number of non-overlapping (mutable) chunks
  • Send the chunks to your threads
  • Fill the chunks independently
  • Join the threads

Sadly, the Image crate which you are using does not provide a lot of support for the first part (splitting the image). So you will need to approach it in one of the following ways:

  • Write the image-splitting operations that you need yourself (this may involve unsafe code, in which case caution must be exercised), and if you have some extra time submit them as a patch to the image crate in order to receive eternal community gratitude.
  • Treat the image as a 1D slice of subpixels (which, in the case of your grayscale image, are equivalent to pixels), and split that slice however you like. The image crate provides native support for this, but you will then need to map between 2D pixel coordinates and 1D pixel indices via the idx = y * width + x formula yourself.

If you decide to go for the later route, here is an example of how to reinterprete the image as a slice of pixels and split that slice into mutable chunks (here individual rows):

// Interprete the image as a slice of pixels via the Deref trait...
// (You do not need to separate this step, I did it just for clarity)
let subpixels: &mut [_] = &mut img;

// ...then *either* get an iterator into blocks of pixels that make
// sense to you (here image rows)...
let mut rows_iter = subpixels.chunks_mut(WIDTH);  // A row is WIDTH pixels

// ...*or* split the image into sub-images recursively (if you go for
// this, make sure that your sub-images are split on row boundaries,
// as doing otherwise would make index computations a nightmare).
let mut (first_half, second_half) = subpixels.split_at_mut(subpixels.len() / 2);

// You may then distribute your pixel chunks to the threads, along with
// some metadata that identifies where the top-left corner of your chunk
// lies in the image, and do your processing.

Once you have figured out your approach for splitting the image, note that you can avoid distributing the work across threads yourself by using the rayon crate for this purpose. I have recently provided someone else with some guidance on doing so in another thread, probably you could use that code as inspiration.


If you decide to stick with manually managed threads, note that you will also need to resolve a lifetime issue: in the eye of the Rust compiler, threads have a potentially infinite lifetime, so it is not legal to pass them references to stack-allocated data like you are doing here, because it might result in a dangling pointer if the thread ends up outliving the image buffer.

The most elegant way to resolve this issue is to use scoped thread APIs, which guarantee that threads will be joined before the underlying scope is exited. Crossbeam and Rayon are two popular parallelization crates which provide scoped threads. Again, the thread which I linked to above provides some examples of how this can work.

3 Likes

Thanks I initially tried using Arc + Mutex, but id didn't work..after trying again I realize I forgot to clone it :man_facepalming:

@HadrienG Thanks for the detailed answer and crate suggestions :slight_smile:

guess I'm still coming around to the rust way of doing things; but loving it so far! makes you re-evaluate how you approach a problem.

I don't understand why we need this line:

let img_arc_clone = img_arc.clone();

can't wen move img_arc into the inner closure just like how we move a Arc value into a closure?

If you were to move img_arc into the inner closure, then you wouldn't be able to spawn more than 1 thread.

fn main() {
    let arc = std::sync::Arc::new(String::from("Hello World"));

    (0..5).for_each(move |x| {
        println!("x is: {:02}, {}", x, arc);

        let arc_a = arc.clone();
        (0..3).for_each(move |y| {
            // I can't move arc into this closure
            println!("x: {}, y: {}, str: {}", x, y, arc_a);
        });
    });
}

what about the arc variable, looks like that I can move arc to the first closure multiple times? what happend behind move?

Thanks

It's moved just once, when the compiler creates (synthesizes) a struct representing your closure. So in essence, the desugared code looks like (rough approximation):

struct __OuterClosure {
     arc: Arc<String>,
}

impl FnMut<i32> for __OuterClosure {
    type Output = ();

    fn call_mut(&mut self, args: i32) -> Self::Output {
        let arc_a = self.arc.clone();
        (0..3).for_each(__InnerClosure {x: args, arc_a});
    }
}

struct __InnerClosure {
    x: i32,
    arc_a: Arc<String>,
}

impl FnMut<i32> for __InnerClosure {
    type Output = ();

    fn call_mut(&mut self, args: i32) -> Self::Output {
         println!("x: {}, y: {}, str: {}", self.x, args, self.arc_a);
    }
}

fn main() {
    let arc = std::sync::Arc::new(String::from("Hello World"));

    (0..5).for_each(__OuterClosure { arc });
}
1 Like

Thanks, your explain is very clear, thank you very much:grinning: