Value moved into closure in previous iteration of loop-CodePudding

I'm trying to use parallelization in a small raytracer I'm implementing.

The idea is that I generate a image pixel by pixel using a function cast_ray(), so I would like to spawn a new thread for each pixel to speed up calculation.

Basically, I am computing an 2D array of a given size. I uses Messages to update an array with the color values.

My renderer function looks like this (I deleted some parts for clarity):


pub fn render_parallel(scene: Scene) -> Vec<Vec<(u8, u8, u8)>> {
    // A small 100x100 2D array
    let mut image = vec![vec![(0, 0, 0); 100]; 100];

    let (tx, rx) = mpsc::channel();

    for x in -50..50 {
        for y in -50..50 {
            // Clone the message emitter
            let tx_cell = tx.clone();
            
            // Spawn a new thread for the pixel
            thread::spawn(move || {
                let ray = ... // A new ray from the camera ;
                let color = cast_ray(
                    &ray, 
                    &scene);

                // Send the coordinates & values to the receiver
                tx_cell.send((
                        ((24 - y) as usize, (25   x) as usize),
                        (color.r, color.g, color.b)))
                    .unwrap();
            });
        }
    }

    for ((i, j), (r, g, b)) in rx {
        image[i][j] = (r, g, b)
    }

    image
}

pub struct Scene {
    pub primitives: Vec<Box<dyn Primitive   Sync>>,
    pub lights: Vec<Box<dyn Light   Sync>>,
}

pub trait Primitive : Send   Sync {
    // Some functions...
}

pub trait Light: Send   Sync {
    // Some functions...
}

The thing is, it doesn't work, and Rust is throwing me the following error :

413 | pub fn render_parallel(scene: Scene) -> Vec<Vec<(u8, u8, u8)>>{
    |                        ----- move occurs because `scene` has type `Scene`, which does not implement the `Copy` trait
...
421 |             thread::spawn(move || {
    |                           ^^^^^^^ value moved into closure here, in previous iteration of loop
...
428 |                     &scene,
    |                      ----- use occurs due to use in closure

I don't understand why the value is moved when I'm only using a reference to it. My other version of render() without using threads is the same as above, without the tx, rx and thread::spawn() and doesn't run into any issue, which confuses me more.

How can I use the scene variable in multiple simultaneous closures (if I'm understanding the problem correctly)?

CodePudding user response：

scene is moved into the closure because you specify move. To borrow scene, you need to rebind it as a reference before you spawn the thread, for example:

let tx_cell = tx.clone();
let scene = &scene;

This rebinds scene in that scope to be a reference to the outer scene. The reference is then moved into the closure.

However, this will fail because thread::spawn() expects a 'static closure (one that does not borrow from its environment) because there is no guarantee the thread will terminate before the value being borrowed is destroyed. In your code, this should not happen because you exhaust rx before scene would be dropped, but the type system doesn't know this.

Instead, consider wrapping scene in an Arc. This gives you thread-safe shared ownership of scene. You can then clone the Arc handle and give a different handle to each thread; each handle will reference the same Scene value.

Insert this as the first line of the function:

let scene = Arc::new(scene);

Then clone the Arc when you clone tx:

let tx_cell = tx.clone();
let scene = Arc::clone(scene);

&scene in your closure will auto-deref the Arc, so you don't need to change that.

Another approach would be to use crossbeam's scoped threads which permit non-static borrows by guaranteeing that spawned threads must terminate before the scope is destroyed.

If you take this approach, note that you should move the for loop into the scope so that it can receive messages in parallel as the spawned threads send them. If you place it after the scope, the threads will send all messages and terminate before you start to process the messages, which means all of the messages must be held in memory at one time.

As a side note, there is a bug in your code that will cause a deadlock: tx lives while you read from rx, so the loop reading from rx will never terminate as there is an open sender that could be used to send more messages. Add drop(tx); before the for loop to close your thread's local sender.