0

I have implemented a chat server which stores the connected users and if a user send a message, the server echoes it to all other clients.

I have C++ background so I made a global static mut USERS:Vec<TcpStream> variable to store and access the users.

I'm handling the connected users from main() like this:

#[async_std::main]
async fn main() -> io::Result<()>{
    let listener = TcpListener::bind("127.0.0.1:14530").await?;

    loop {
        let (stream, addr) = listener.accept().await?;
        
        unsafe {
            USERS.push(stream.clone());
        }
        
        task::spawn(on_connection(stream, addr));
    }
}

and on_connection function is:

async fn on_connection(mut stream:TcpStream, addr:SocketAddr) -> io::Result<()> {
    println!("New Connection: {}", addr.to_string());

    let mut buffer = [0u8; 1024];
    loop {
        let len = stream.read(&mut buffer).await?; 

        if len > 0 {
            print!("Message from {} => {}", addr.to_string(), String::from_utf8_lossy(&buffer));
            
            unsafe {
                for mut user in USERS.clone() {
                    user.write(&buffer).await?;
                }
            }
        }
        else {
            println!("Disconnected: {}", addr.to_string());
            
            break
        }
    };

    Ok(())
}

Is it okay to use Rust like this?

I want to make the application safe and use Rust's safe environment without "unsafe". But I couldn't work out how to store global users to access later without unsafe.

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
eminfedar
  • 558
  • 3
  • 16
  • I use DashMap and pass it around to fns that need it – J. Doe Nov 24 '21 at 12:58
  • 1
    No, it is not safe. Not sure why `clone()` is used on the vector, but that will not fix the race condition either. – justinas Nov 24 '21 at 13:05
  • @J.Doe so should I create a local Vec in `main()` and pass it every function as a parameter? – eminfedar Nov 24 '21 at 13:53
  • 2
    Best is probably to use an `Arc>>` and pass it to every function. – Jmb Nov 24 '21 at 13:57
  • 1
    Never, ever, use `static mut`. If you know what you're doing, use `static` with `UnsafeCell`. If you don't (which means "you're not a Rust expert", not "you haven't program in C++"), don't use `unsafe` at all. – Chayim Friedman Nov 24 '21 at 23:50
  • 1
    I don't know if async-std is multithreaded: if yes, you probably need `Mutex` (and can leave it as global), if not, you only need to create it in `main()` and pass it downwards. – Chayim Friedman Nov 24 '21 at 23:52
  • As far as I see on `htop` task manager, it spawns as many threads as number of cores. So it is multithreaded I suppose (probably a thread-pool) – eminfedar Nov 25 '21 at 10:32

1 Answers1

0

Considering the comments, this is my "safe version" of the implementation above:

use std::net::SocketAddr;
use async_std::net::{TcpStream, TcpListener};
use async_std::sync::{Arc, Mutex};
use async_std::io::{ReadExt, WriteExt, Result};
use async_std::task;


async fn on_connection(mut stream:TcpStream, addr:SocketAddr, users:Arc<Mutex<Vec<TcpStream>>>) -> Result<()> {
    println!("New Connection: {}", addr.to_string());

    let mut buffer = [0u8; 1024];
    loop {
        let len = stream.read(&mut buffer).await?; 

        if len > 0 {
            print!("{} => {}", addr.to_string(), String::from_utf8_lossy(&buffer));

            let users = users.lock().await;
            
            for mut user in &*users {
                // send everyone except itself
                if user.peer_addr()? != stream.peer_addr()? {
                    user.write(&buffer).await?;
                }
            }
        }
        else {
            println!("Disconnected: {}", addr.to_string());
            break;
        }
    };

    Ok(())
}

#[async_std::main]
async fn main() -> Result<()>{
    let listener = TcpListener::bind("127.0.0.1:14530").await?;
    let users = Vec::new();
    let arc_users = Arc::new(Mutex::new(users));
    
    loop {
        let users = arc_users.clone();
        let (stream, addr) = listener.accept().await?;

        let mut write_permission= users.lock().await;
        write_permission.push(stream.clone());
        drop(write_permission);
        
        task::spawn(on_connection(stream, addr, users));
    };
}
eminfedar
  • 558
  • 3
  • 16