StdoutLock is unsound

[LunaBorowska]

Run the following program:

use async_std::io::prelude::WriteExt;
use async_std::{io as async_io, task};
use std::io::{self, Write};
use std::thread;

#[async_std::main]
async fn main() -> io::Result<()> {
    let mut async_locked = async_io::stdout().lock().await;
    let fut = task::spawn_blocking(|| {
        let stdout = io::stdout();
        let mut locked = stdout.lock();
        writeln!(locked, "sync stdout in thread {:?}", thread::current())?;
        locked.flush()?;
        Ok(())
    });
    async_std::writeln!(
        async_locked,
        "async stdout in thread {:?}",
        thread::current(),
    )
    .await?;
    async_locked.flush().await?;
    fut.await
}

Cargo.toml dependencies

[dependencies]
async-std = { version = "1.6.0", features = [ "attributes", "unstable" ] }

This program violates the invariant of StdoutLock (there must never be two instances of StdoutLock owned by different threads), leading to UB and panics like the following.

sync stdout in thread Thread { id: ThreadId(10), name: None }
thread 'main' panicked at 'already borrowed: BorrowMutError', /rustc/8d69840ab92ea7f4d323420088dd8c9775f180cd/src/libcore/cell.rs:878:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Considering lock is an unstable API, I would recommend removing the API, and suggest usage of spawn_blocking instead. I don't think it's possible for async StdoutLock to ever be sound with threaded executors

[yoshuawuyts]

Hi, thanks for reporting. You're right the current implementation isn't correct. We had a PR to remove it a while ago, but had to revert it because it included additional changes that caused regressions.

The right approach here now indeed seems to remove the unstable stdio types.