`UnsafeCell` shared accross threads with no locking - this can cause UB right?

Question

Consider the following code:

use std::{cell::UnsafeCell, io, net::TcpStream, sync::Arc};

use native_tls::TlsStream;

#[derive(Debug)]
pub struct TcpStreamRecv(Arc<UnsafeCell<TlsStream<TcpStream>>>);

unsafe impl Send for TcpStreamRecv {}
unsafe impl Sync for TcpStreamRecv {}

impl io::Read for TcpStreamRecv {
    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
        unsafe { &mut *self.0.get() }.read(buf)
    }
}

#[derive(Debug)]
pub struct TcpStreamSend(Arc<UnsafeCell<TlsStream<TcpStream>>>);

unsafe impl Send for TcpStreamSend {}
unsafe impl Sync for TcpStreamSend {}

impl io::Write for TcpStreamSend {
    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
        unsafe { &mut *self.0.get() }.write(buf)
    }

    fn flush(&mut self) -> io::Result<()> {
        unsafe { &mut *self.0.get() }.flush()
    }
}

pub fn tcp_split(stream: TlsStream<TcpStream>) -> (TcpStreamSend, TcpStreamRecv) {
    let inner = Arc::new(UnsafeCell::new(stream));
    let send = TcpStreamSend(inner.clone());
    let recv = TcpStreamRecv(inner);
    (send, recv)
}

My reasoning is as folows:

Safe code can obtain a TcpStreamSend and a TcpStreamRecv with the same underlying TlsStream.

If each of these is sent to a separate thread, it is possible to call TcpStreamSend::write and TcpStreamRecv::read at the same time. Each of these functions obtains a &mut reference to the underlying TlsStream.

Therefore, since it is illegal to have 2 mutable references at the same time, this code can cause UB and should be considered unsound. Is this correct?

My coworker assured me that "if it works, it works", and the code indeed appears to function normally most of the time, apart from some random occasional panics. In my understanding, however, it can cause unpredictable problems anywhere in our codebase, and should be rewritten immediately. Did I miss something?

By the way, I did some research and the code seems to be inspired by this SO answer which is more convoluted, but, as far as I can see, equally bad.

Answer 1

Definitely unsound.

Simply having two overlapping &mut references in existence at the same time in Rust is undefined behavior, which will happen with this if two threads call read and write at the same time. This is a problem even for non-threaded code - Rust assumes that no &mut references can alias.

Splitting a TlsStream without a lock like this is just not possible. Even if the read and write implementations in TcpStream are independent, TlsStream may need to alter how it writes based on messages from the server (ex. rekeying), which will require that the reader alter state that the writer uses.