Move issue with clone / Thread on Arc Vector of u8

I have code like so:

let (tx, rx) = spmc::channel::<Arc<Vec<u8>>>(); 
handles = Vec::new();
        for n in 0..THREAD_COUNT
        {
            
            handles.push(thread::spawn(move || {
                let ed = collect_args();
                let rx = rx.clone();

// ....

I am getting

error[E0382]: capture of moved value: rx
–> src/main.rs:68:26
|
66 | handles.push(thread::spawn(move || {
| ------- value moved (into closure) here
67 | let ed = collect_args();
68 | let rx = rx.clone();
| ^^ value captured here after move
|
= note: move occurs because rx has type spmc::Receiver<std::sync::Arc<std::vec::Vec<u8>>>, which does not implement the Copy trait
Even though I’m not using Copy, I’m using clone so that is where I am confused with this. Although it seems to work with a Arc of String type.

You need to clone rx outside the closure since it’s moving rx into it, preventing its use across loop iterations. Something like:

for n in 0..THREAD_COUNT
        {
            let rx_clone = rx.clone();            
            handles.push(thread::spawn(move || {
                let ed = collect_args();
                // use `rx_clone` here
3 Likes

Ah thanks, that’s tricky to figure out/remember.

I got a related question - if I have a loop in Rust and I need to pass local arguments into a function on each loop iteration, what is the preferred method in rust? I thought I could do this with using foo(&the_arg) but the_arg still seems to get “moved” into the function on each loop iteration and thus won’t compile. I’m confused as to what the proper way to accomplish this task is in Rust. I had assumed that since &the_arg is an immutable reference, it would not be moved. Seems to be the case at least with Vector of Regex because “Copy” trait isn’t implemented, or at least that’s what the compiler says is the problem.

Even if I try to make a clone, then that clone gets moved and fails on subsequent iterations as well.

Working with closures effectively, particularly capturing things correctly, takes a bit of practice.

I’m on mobile so will give you a few quick examples - feel free to ask followups.

Let me also say that I find it helpful to keep in mind that closures are desugared into structs; values captured from outside the closure are stored in fields of this struct (this isn’t strictly true for some things, like using static values, but this isn’t important here).

fn by_ref<T>(_: &T) {}

fn main() {
  let v = vec![1, 2, 3];
  for _ in 0..5 {
     // this moves `v` into the closure,
     // and we can’t use it across iterations
     // this wouldn’t compile due to that
     let c = move || by_ref(&v);
     // this closure captures by ref because 
     // there’s no `move`
     let c2 = || by_ref(&v);
     let ref_to_v = &v;
     // `move` closure follows but it’s 
     // moving a reference, and references are 
     // `Copy` types - this is A-ok
     let c3 = move || by_ref(ref_to_v);

   }
}

So c above is like:

struct Closure {
   v: Vec<i32>, // we own the value as it was moved here
}

// super simplified impl of a closure trait to 
// illustrate what reference is taken
impl Fn for Closure {
   fn call(&self) {
      // now we take a reference to our own value
      by_ref(&self.v)
   }
}

Whereas c2 and c3 would be:

struct Closure<'a> {
   // stores a ref since that was the type we `moved` or captured
   v: &'a Vec<i32>,
}

I’m glossing over some things that aren’t super important to the main point (and I’m lazy on the phone :slight_smile:). But do ask if you have questions!

Generally the compiler will try to borrow from the environment, unless the semantics of the closure require a move (eg closure calls drop(v)) or it’s forced to move explicitly. If a closure needs to move some things and borrow others, using an approach like c3 is the solution - explicitly move a reference for things that need to be only borrowed.

3 Likes

Wow thanks for this!

I finally solved my problem… What was happening was that I was doing a clone but it was outside of the for loop which was to pass the clone into each new thread. The problem with this was that each new thread needs it own new clone of the data and instead, it was trying to have all new threads use just the one clone… This was in turn causing it to be “moved” to the first thread and thus throwing the error.

Rust can be tricky in this way because there are some situations where you must lift the code out of the loop and out of the closure, but there are other situations where you actually need to place the code down into the loop or inside the closure.

I wouldn’t necessarily blame Rust, it’s probably more of a thing inherent to programming in general and especially concurrency but Rust gives you the luxury of identifying the problem at compile-time instead of spending hours in GDB. Nevertheless, minute changes of where assignments occur can be difficult to figure out at times… Spending hours only to realize a let statement needed to be moved a few lines up or down never ceases to amaze me haha.A

Thanks again.

I like to take advantage of the fact that everything is an expression, and couple the clones to the move closure using an anonymous block. That ensures the cloned rx is scoped appropriately, and semantically tells readers that the cloning is connected to the closure.

In this case, it would look like:

for n in 0..THREAD_COUNT {
    handles.push(
        thread::spawn({
            let rx = rx.clone();
            move || {
                let msg = rx.recv(); 
            }
        })
    )

Note: since the scope is so limited, it's perfectly fine to reuse the rx name. Outside the anonymous block, the original rx still exists. It also makes refactoring easier, as you can copy and paste the whole block around as a unit.

2 Likes

Yup, this is good advice and something I do in my own code as well. I omitted this part partly to keep things slightly simpler and more straightforward, and partly to minimize the amount of typing on my phone :slight_smile:. Thanks for mentioning it though.