Ben Ruijl
Ben Ruijl

Reputation: 5143

Sharing mutable self between multiple threads

I have a server that accepts connections from multiple clients. Each client could send a message to the server, which is broadcast to all other clients. The problem is that the function that handles each connection should have a reference to the server. However, I want to handle the connections in separate threads, so I cannot use a reference directly.

Since scoped is deprecated, I tried wrapping self in an Arc, but more problems ensued. Below is my attempt:

struct Server {
    listener: TcpListener,
    clients: Vec<TcpStream>
}

impl Server {

    fn new() -> Server {
        Server { 
            listener : TcpListener::bind("127.0.0.1:8085").unwrap(),
            clients : vec![] 
        }
    }

    fn handle(&self) {
        println!("test");
    }

    fn start(mut self) {
        let mut handles = vec![];
        let a : Arc<Mutex<Server>> = Arc::new(Mutex::new(self));
        let mut selfm = a.lock().unwrap();

        // cannot borrow as mutable... ?
        for stream in selfm.listener.incoming() {
            match stream {
                Ok(stream) => {
                    selfm.clients.push(stream);
                    let aa = a.clone();
                    handles.push(thread::spawn(move || {
                        aa.lock().unwrap().handle();
                    }));
                },
                Err(e) => { println!("{}", e); },
            }
        }
    }

Rust Playground

I don't understand what to do anymore, and I fear deadlocks will arise with all these locks. Do you have any suggestions?

Upvotes: 4

Views: 1181

Answers (1)

fjh
fjh

Reputation: 13091

The error is pretty much unrelated to having multiple threads. The issue is, as the compiler says, that selfm is already borrowed in the line

for stream in selfm.listener.incoming() {

so it cannot be mutably borrowed in the line

selfm.clients.push(stream);

One way to fix this is to destructure selfm before the loop, so the borrows don't conflict. Your start method will then look as follows:

fn start(mut self) {
    let mut handles = vec![];
    let a : Arc<Mutex<Server>> = Arc::new(Mutex::new(self));
    let mut selfm = a.lock().unwrap();

     // destructure selfm here to get a reference to the listener and a mutable reference to the clients
    let Server { ref listener, ref mut clients} = *selfm;
    for stream in listener.incoming() { // listener can be used here
        match stream {
            Ok(stream) => {
                clients.push(stream); // clients can be mutated here
                let aa = a.clone();
                handles.push(thread::spawn(move || {
                    aa.lock().unwrap().handle();
                }));
            },
            Err(e) => { println!("{}", e); },
        }
    }
}

(That being said, you're right to be concerned about the locking, since the mutex will remain locked until selfm goes out of scope, i.e. only when start terminates, i.e. never. I would suggest an alternative design, but it's not really clear to me why you want the threads to have access to the server struct.)

Upvotes: 1

Related Questions