Borrow variable into numerous threads

[leroy]

Hey, I'm making a TCP Server. I want to store the std::net::TcpStream of each connection inside a Vector. I'm attempting to make it so all responses instantly get logged to stdout and then I can use the send command to send a message to the client by specify the position of the TcpStream in the Vector (client_id being Vector position). I cant move connections into the thread as it's need to be globally accessible, I wanted it so that when a client connects to the server a TcpStream is pushed into the connections Vector, handle_client is also run to log received messages. Input is also handled in another thread so I assume I'd have to borrow connections into that thread so I can use it if the list or send command is used. I don't believe I can borrow it mutably numerous time though. How would I go about doing this?

use std::net::{TcpListener, TcpStream};
use std::io;
use std::io::{Read, Write};
use std::thread;

fn handle_client(mut stream: &TcpStream) {
    println!("Client connected.");

    loop {
        let mut data = [0 as u8; 1024];
        stream.read(&mut data).unwrap();

        let string = String::from_utf8_lossy(&data);
        println!("{}", string);
    }
}

fn main() {
    println!("Starting Listener on http://127.0.0.1:5555");

    let listener= TcpListener::bind("127.0.0.1:5555")
        .unwrap();

    thread::spawn(move || {
        loop {
            run_command(accept_input()); 
        } 
    });

    let mut connections: Vec<TcpStream> = Vec::new();

    for stream in listener.incoming() {
        match stream {
            Ok(s) => {
                println!("New Connection: {}", s.peer_addr().unwrap());
                
                thread::spawn(|| {
                    let pos = connections.len();
                    connections.push(s);

                    handle_client(&connections[pos + 1]);
                });
            }
            Err(err) => println!("Failed to accept incoming connection: {}", err),
        };
    }
}

fn accept_input() -> String {
    print!("Server > ");
    io::stdout().flush().expect("Failed to flush stdout");
   
    let mut input = String::new();
    io::stdin().read_line(&mut input).unwrap();
    
    input
}

fn run_command(cmd: String) {
    match cmd.as_str().trim() {
        "list" => println!("list clients"),
        "send" => println!("send <client_id> <string>"),
        "help" => println!("list: List clients\nsend <client_id> <string>: Send message to client\nhelp: Provides a help menu"),
        _ => println!("Unknown command"),
    }
}

[kornel]

When a variable is used in more than one place you have to wrap it in Arc and make a clone of it for each thread.

let connections = Arc::clone(&connections);
thread::spawn(move || {

To mutate something wrapped in Arc you need Mutex, so:

let mut connections: Arc<Mutex<Vec<TcpStream>>> = Arc::default();

[steffahn]

@kornel

To mutate something wrapped in Arc you need Mutex, so:

let mut connections: Arc<Mutex<Vec<TcpStream>>> = Arc::default();

@leroy

Given that, AFAICT, there’s a long-running handle_client function that (currently) expects a &TcpStream reference into this Vec, let me note that there would need to be further refactoring to avoid locking the Mutex for a long time (i.e. make sure that multiple handle_client calls can run in parallel; e.g. with another layer of Arc, or by cloning the TcpStream itself [for the latter choice I’m not entirely sure what possible error cases and/or overhead TcpStream::try_clone in the standard library can run into]).

I can’t really make good suggestions as I have no idea what the Vec is even used for in OP’s original code; looks like items are just placed into it for no good reason and then left there. Also the index in &connections[pos + 1] seems off.