Concurrent channel usage not thread safe? #322
Description
While trying to determine whether bk-rs/ssh-rs#50 is a bug in async-ssh2-lite, in ssh2-rs, or in libssh2 I tried to convert my reproducing example to be sync/using ssh2 directly, and doing so I hit another issue which to me smells like it could be symptoms of the crate's usage of libssh2 not being entirely thread-safe.
I suspect that the issue here is that on drop libssh2_channel_free is called without locking the session, and which then tries to close the channel internally since it wasn't explicitly closed first, while some other thread has the lock held and fiddles with the session. Adding an explicit session.close().unwrap() seems to resolve it.
Reproducing
The setup is the same as in the linked issue except with the following code instead:
Client code
use std::{
io::{Read, Write},
net::TcpStream,
path::PathBuf,
sync::Arc,
thread::{self, sleep},
time::Duration,
};
use ssh2::Session;
fn main() {
let mut session = Session::new().unwrap();
session.set_tcp_stream(TcpStream::connect("<SERVER-IP>:22").unwrap());
session.handshake().unwrap();
let private_key = PathBuf::from("/whatever/path");
session
.userauth_pubkey_file("<SERVER-USER>", None, &private_key, None)
.unwrap();
let session = Arc::new(session);
thread::scope(|scope| loop {
println!();
scope.spawn(|| send_request(&session, "A", 8001, "a"));
scope.spawn(|| send_request(&session, "A", 8001, "a"));
scope.spawn(|| send_request(&session, "B", 8002, "b"));
sleep(Duration::from_secs(5));
});
}
fn send_request(session: &Session, name: &'static str, port: u16, expected: &str) {
let mut stream = session
.channel_direct_tcpip("127.0.0.1", port, None)
.expect("couldn't open direct-tcpip channel");
stream
.write_all("GET / HTTP/1.0\r\n\r\n".as_bytes())
.unwrap();
let mut buf = String::new();
stream
.read_to_string(&mut buf)
.expect("failed to read response");
let res = buf.split_once("\r\n\r\n").expect("got invalid response").1;
eprintln!("{name}: {res}");
assert_eq!(res, expected);
}Expected
You get a stream of output where each A line has a response of a and each B line has a response of b.
Actual
A: a
B: b
thread '<unnamed>' panicked at src/bin/sync.rs:44:10:
failed to read response: Custom { kind: Other, error: "transport read" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace