Skip to content

Commit

Permalink
fix: flush DataChannelCmd::StartForward* commands (rapiz1#316)
Browse files Browse the repository at this point in the history
Without flushing this may sit in a kernel buffer and we won't
know if the channel is still alive. This is particularly problematic
for the TCP connection pool.
  • Loading branch information
RyanAD authored Feb 14, 2024
1 parent 915bf4d commit 6322102
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
14 changes: 13 additions & 1 deletion src/helper.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use anyhow::{anyhow, Result};
use anyhow::{anyhow, Context, Result};
use async_http_proxy::{http_connect_tokio, http_connect_tokio_with_basic_auth};
use backoff::{backoff::Backoff, Notify};
use socket2::{SockRef, TcpKeepalive};
use std::{future::Future, net::SocketAddr, time::Duration};
use tokio::io::{AsyncWrite, AsyncWriteExt};
use tokio::{
net::{lookup_host, TcpStream, ToSocketAddrs, UdpSocket},
sync::broadcast,
Expand Down Expand Up @@ -144,3 +145,14 @@ where
}
}
}

pub async fn write_and_flush<T>(conn: &mut T, data: &[u8]) -> Result<()>
where
T: AsyncWrite + Unpin,
{
conn.write_all(data)
.await
.with_context(|| "Failed to write data")?;
conn.flush().await.with_context(|| "Failed to flush data")?;
Ok(())
}
13 changes: 4 additions & 9 deletions src/server.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::config::{Config, ServerConfig, ServerServiceConfig, ServiceType, TransportType};
use crate::config_watcher::{ConfigChange, ServerServiceChange};
use crate::constants::{listen_backoff, UDP_BUFFER_SIZE};
use crate::helper::retry_notify_with_deadline;
use crate::helper::{retry_notify_with_deadline, write_and_flush};
use crate::multi_map::MultiMap;
use crate::protocol::Hello::{ControlChannelHello, DataChannelHello};
use crate::protocol::{
Expand Down Expand Up @@ -498,14 +498,9 @@ struct ControlChannel<T: Transport> {

impl<T: Transport> ControlChannel<T> {
async fn write_and_flush(&mut self, data: &[u8]) -> Result<()> {
self.conn
.write_all(data)
write_and_flush(&mut self.conn, data)
.await
.with_context(|| "Failed to write control cmds")?;
self.conn
.flush()
.await
.with_context(|| "Failed to flush control cmds")?;
Ok(())
}
// Run a control channel
Expand Down Expand Up @@ -640,7 +635,7 @@ async fn run_tcp_connection_pool<T: Transport>(
'pool: while let Some(mut visitor) = visitor_rx.recv().await {
loop {
if let Some(mut ch) = data_ch_rx.recv().await {
if ch.write_all(&cmd).await.is_ok() {
if write_and_flush(&mut ch, &cmd).await.is_ok() {
tokio::spawn(async move {
let _ = copy_bidirectional(&mut ch, &mut visitor).await;
});
Expand Down Expand Up @@ -690,7 +685,7 @@ async fn run_udp_connection_pool<T: Transport>(
.recv()
.await
.ok_or_else(|| anyhow!("No available data channels"))?;
conn.write_all(&cmd).await?;
write_and_flush(&mut conn, &cmd).await?;

let mut buf = [0u8; UDP_BUFFER_SIZE];
loop {
Expand Down

0 comments on commit 6322102

Please sign in to comment.