Multiple threads writting on the same shared data

I want to have many threads doing computations and from time to time writing on the same structure in a way that's safe.
Since I'm just incrementing counters, I thought RwLock would do the trick.
I tried to use an Arc for the shared data and get_mut but the code panics (after following @Cerberuser 's suggestion, the code compiles but panics)

thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value

It seems once one thread gets the mutable reference, it's not available for the others.

I understand that all threads getting a reference to the same mutable object wouldn't be safe, but it's the first time I'm trying to do concurrency in rust and I'm not sure what is it that I should do instead, any ideas?

Here's the code I'm using. I removed most of the unnecessary things for the example, but, sorry, I guess I could have simplified further.

use std::sync::RwLock;
use std::sync::Arc;
use std::thread;

struct Filter {
    descenand_a: Option<Box<Filter>>,
    descenand_b: Option<Box<Filter>>,
    particle_counter_a: RwLock<u32>,
    particle_counter_b: RwLock<u32>
}

impl Clone for Filter {
    fn clone(&self) -> Filter {
        Filter {
            descenand_a: self.descenand_a.clone(),
            descenand_b: self.descenand_b.clone(),
            particle_counter_a: RwLock::new(*self.particle_counter_a.read().unwrap()),
            particle_counter_b: RwLock::new(*self.particle_counter_b.read().unwrap())
        }
    }
}

impl Filter {

    pub fn new(descenand_a: Option<Box<Filter>>, descenand_b: Option<Box<Filter>>) -> Filter {
        Filter{descenand_a, descenand_b, particle_counter_a: RwLock::new(0), particle_counter_b: RwLock::new(0)}
    }
    
    pub fn get_results(&self) -> Vec<u32> {
        let mut vec = Vec::new();
        match &self.descenand_a {
            Some(x) => vec.append(&mut x.get_results()),
            None => vec.push(*self.particle_counter_a.read().unwrap()),
        }
        match &self.descenand_b {
            Some(x) => vec.append(&mut x.get_results()),
            None => vec.push(*self.particle_counter_b.read().unwrap()),
        }
        vec
    }

}

trait Particle {
    fn decide_way(&self) -> bool;
    fn pass_filter(&mut self, filter: &mut Filter) {
        if self.decide_way() {
            *filter.particle_counter_a.write().unwrap() += 1;
            if let &mut Some(ref mut x) = &mut filter.descenand_a {
                self.pass_filter(x);
            }
        } else {
            *filter.particle_counter_b.write().unwrap() += 1;
            if let &mut Some(ref mut x) = &mut filter.descenand_b {
                self.pass_filter(x);
            }
        }
    }
}

struct StupidParticle;

impl Particle for StupidParticle {
    fn decide_way(&self) -> bool {
        true
    }
}

fn main() {
    let mut filter = Arc::new(Filter::new(None, None));
    let mut handles = vec![];
    for _ in 0..10 {
        let mut p = StupidParticle {};
        let mut fc = Arc::clone(&filter);
	    handles.push(thread::spawn(move || {
		    p.pass_filter(Arc::get_mut(&mut fc).unwrap());
	    }));
	}
	println!("results: {:?}", (&filter).get_results());

}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=8722e30b17bf6d347e92fcf7beae0b68

You're moving filter into closure and then clone it inside, but you probably want to clone it outside of the closure and move the clone:

fn main() {
    let filter = Arc::new(Filter::new(None, None));
    for _ in 0..10 {
        let mut p = StupidParticle {};
+       let mut fc = filter.clone();
	    thread::spawn(move || {
-	        let mut fc = filter.clone();
		    p.pass_filter(Arc::get_mut(&mut fc).unwrap());
	    });
	}
}

When I make this change on your playground, it panics, so I'm not sure what else should be changed, but this - "clone before thread::spawn, not inside" - is a rather common idiom for such sharing anyway.

1 Like

yes, that and changing to Arc:clone did it, thanks!

        let mut fc = Arc::clone(&filter);

But as you say it panics, I'll update the code and the question.

Change to p.pass_filter(&*fc); and remove lots of muts. Also need to join handles before result.

Atomics are an alternative to RwLock for what you show; requires some reading to know if useful(or not) for what you intend.

Well, the reason for panic seems to be simple. Quoting the updated starting post:

get_mut is not for the shared data - quoting documentation (emphasis mine):

Returns a mutable reference into the given Arc , if there are no other Arc or Weak pointers to the same allocation.

So, you have to use shared references to Arc and some internally-mutable wrapper - e.g. RwLock, as it is in your code.

1 Like

Yes, thanks, the key part was removing the mut, I assumed I needed that since I'm modifying the counters, but with RwLock or AtomicU32 mutability is not necessary.
That's the part I was missing.
Thanks @Cerberuser for your help too.
This is the corrected code:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=386e37a4e2c5ba7ebd8db387fc5414a4

And with AtomicU32 (seems to be a little bit faster for this case):

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=83b4c40f09329390ac195168cbb3b680

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.