Skip to content

fix: ss2022 throughput, simple-obfs TLS state machine, MMDB path #1330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ibigbug merged 2 commits into from
Apr 25, 2026
Merged

fix: ss2022 throughput, simple-obfs TLS state machine, MMDB path #1330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f624685
some ss and obfs fixes
ibigbug Apr 24, 2026
40522dd
f
ibigbug Apr 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions clash-bin/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,15 @@ fn main() -> anyhow::Result<()> {
recommended to enable this if you are using clash verge."
);
if let Some(dir) = &cli.directory {
std::env::set_current_dir(dir)?;
// Canonicalize to an absolute path before changing the process cwd.
// If `dir` is relative (e.g. `./clash-bin/tests/data/config`),
// calling set_current_dir then passing the same relative string as
// `cwd` to start_scaffold would cause paths like
// `cwd.join("Country.mmdb")` to be resolved from the *new* process
// cwd, doubling the directory segments and producing a path that
// doesn't exist (os error 2).
let abs = std::fs::canonicalize(dir)?;
std::env::set_current_dir(&abs)?;
}
if config.general.mmdb.is_none() {
config.general.mmdb = Some("Country.mmdb".to_string());
Expand All @@ -194,9 +202,19 @@ fn main() -> anyhow::Result<()> {
}
}

// When compatibility mode called set_current_dir the process cwd is
// already correct; pass None so start_scaffold uses "." (= the new cwd)
// rather than the original relative cli.directory which would be resolved
// from the wrong base.
let cwd = if cli.compatibility && cli.directory.is_some() {
None
} else {
cli.directory.map(|x| x.to_string_lossy().to_string())
};

clash::start_scaffold(clash::Options {
config: clash::Config::Internal(config),
cwd: cli.directory.map(|x| x.to_string_lossy().to_string()),
cwd,
rt: Some(TokioRuntime::MultiThread),
log_file: cli.log_file,
})
Expand Down
10 changes: 7 additions & 3 deletions clash-lib/src/app/dispatcher/dispatcher_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ use crate::app::dns::ThreadSafeDNSResolver;

use super::statistics_manager::Manager;

const DEFAULT_BUFFER_SIZE: usize = 16 * 1024;
// SS2022 (AEAD-2022) MAX_PACKET_SIZE is 0xFFFF (65535 bytes). Using a relay
// buffer smaller than that forces the cipher to split every full packet into
// multiple smaller encrypted chunks, multiplying encrypt/decrypt overhead.
Comment on lines +33 to +35
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says SS2022 max packet size is 0xFFFF (65535), but DEFAULT_BUFFER_SIZE is set to 64 * 1024 (= 65536). Either adjust the constant to match the stated limit, or update the comment to explain why a 64 KiB buffer (one byte larger) is chosen (e.g., power-of-two sizing).

Suggested change
// SS2022 (AEAD-2022) MAX_PACKET_SIZE is 0xFFFF (65535 bytes). Using a relay
// buffer smaller than that forces the cipher to split every full packet into
// multiple smaller encrypted chunks, multiplying encrypt/decrypt overhead.
// SS2022 (AEAD-2022) MAX_PACKET_SIZE is 0xFFFF (65535 bytes). We use a 64 KiB
// relay buffer (65536 bytes) as a convenient power-of-two allocation, which is
// still large enough to hold the largest SS2022 packet without forcing the
// cipher to split a full packet into multiple smaller encrypted chunks.

Copilot uses AI. Check for mistakes.
// Classic AEAD ciphers cap at 0x3FFF (16383 bytes) so they are unaffected.
const DEFAULT_BUFFER_SIZE: usize = 64 * 1024;

pub struct Dispatcher {
outbound_manager: ThreadSafeOutboundManager,
Expand Down Expand Up @@ -251,7 +255,7 @@ impl Dispatcher {
*/
let (mut local_w, mut local_r) = udp_inbound.split();
let (remote_receiver_w, mut remote_receiver_r) =
tokio::sync::mpsc::channel(32);
tokio::sync::mpsc::channel(256);

let s = sess.clone();
let ss = sess.clone();
Expand Down Expand Up @@ -363,7 +367,7 @@ impl Dispatcher {

let (mut remote_w, mut remote_r) = outbound_datagram.split();
let (remote_sender, mut remote_forwarder) =
tokio::sync::mpsc::channel::<UdpPacket>(32);
tokio::sync::mpsc::channel::<UdpPacket>(256);

// remote -> local
let r_handle = tokio::spawn(async move {
Expand Down
13 changes: 10 additions & 3 deletions clash-lib/src/proxy/datagram.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ pub struct OutboundDatagramImpl {
resolver: ThreadSafeDNSResolver,
flushed: bool,
pkt: Option<UdpPacket>,
// Pre-allocated receive buffer; avoids a 65535-byte heap allocation on
// every poll_next call.
recv_buf: Vec<u8>,
}

impl OutboundDatagramImpl {
Expand All @@ -82,6 +85,7 @@ impl OutboundDatagramImpl {
resolver,
flushed: true,
pkt: None,
recv_buf: vec![0u8; 65535],
}
}
}
Expand Down Expand Up @@ -190,9 +194,12 @@ impl Stream for OutboundDatagramImpl {
mut self: Pin<&mut Self>,
cx: &mut Context<'_>,
) -> Poll<Option<Self::Item>> {
let Self { ref mut inner, .. } = *self;
let mut mem = vec![0u8; 65535];
let mut buf = ReadBuf::new(&mut mem);
let Self {
ref mut inner,
ref mut recv_buf,
..
} = *self;
let mut buf = ReadBuf::new(recv_buf.as_mut_slice());
match ready!(inner.poll_recv_from(cx, &mut buf)) {
Ok(src) => {
let data = buf.filled().to_vec();
Expand Down
15 changes: 8 additions & 7 deletions clash-lib/src/proxy/shadowsocks/outbound/datagram.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::{
io,
net::SocketAddr,
pin::Pin,
sync::Mutex,
task::{Context, Poll},
};

Expand Down Expand Up @@ -200,16 +201,16 @@ where

/// Shadowsocks UDP I/O that ProxySocket required
pub(crate) struct ShadowsocksUdpIo {
w: tokio::sync::Mutex<SplitSink<AnyOutboundDatagram, UdpPacket>>,
r: tokio::sync::Mutex<(SplitStream<AnyOutboundDatagram>, BytesMut)>,
w: Mutex<SplitSink<AnyOutboundDatagram, UdpPacket>>,
r: Mutex<(SplitStream<AnyOutboundDatagram>, BytesMut)>,
}

impl ShadowsocksUdpIo {
pub fn new(inner: AnyOutboundDatagram) -> Self {
let (w, r) = inner.split();
Self {
w: tokio::sync::Mutex::new(w),
r: tokio::sync::Mutex::new((r, BytesMut::new())),
w: Mutex::new(w),
r: Mutex::new((r, BytesMut::new())),
}
}
}
Expand All @@ -225,7 +226,7 @@ impl DatagramSend for ShadowsocksUdpIo {
buf: &[u8],
target: std::net::SocketAddr,
) -> Poll<io::Result<usize>> {
let mut w = self.w.try_lock().expect("must acquire");
let mut w = self.w.lock().unwrap();
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid panicking on poisoned mutexes in UDP poll paths.

Line 229, Line 247, and Line 259 use lock().unwrap(). If the lock is poisoned, this panics and can tear down active UDP handling instead of returning an I/O error.

Suggested fix 
-        let mut w = self.w.lock().unwrap();
+        let mut w = match self.w.lock() {
+            Ok(guard) => guard,
+            Err(_) => {
+                return Poll::Ready(Err(io::Error::other(
+                    "shadowsocks udp writer lock poisoned",
+                )));
+            }
+        };

-        let mut w = self.w.lock().unwrap();
+        let mut w = match self.w.lock() {
+            Ok(guard) => guard,
+            Err(_) => {
+                return Poll::Ready(Err(io::Error::other(
+                    "shadowsocks udp writer lock poisoned",
+                )));
+            }
+        };

-        let mut g = self.r.lock().unwrap();
+        let mut g = match self.r.lock() {
+            Ok(guard) => guard,
+            Err(_) => {
+                return Poll::Ready(Err(io::Error::other(
+                    "shadowsocks udp reader lock poisoned",
+                )));
+            }
+        };

Also applies to: 247-247, 259-259

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@clash-lib/src/proxy/shadowsocks/outbound/datagram.rs` at line 229,
Occurrences of let mut w = self.w.lock().unwrap() in datagram.rs will panic on a
poisoned mutex; change each to handle LockResult by matching or using .map_err
to convert a poisoned lock into a recoverable io::Error (e.g., return
Err(std::io::Error::new(std::io::ErrorKind::Other, "mutex poisoned"))) so UDP
poll paths return an I/O error instead of panicking; update all three sites (the
three occurrences of self.w.lock().unwrap()) to propagate a std::io::Error when
the lock is poisoned.

match w.start_send_unpin(UdpPacket {
data: buf.to_vec(),
src_addr: SocksAddr::any_ipv4(),
Expand All @@ -243,7 +244,7 @@ impl DatagramSend for ShadowsocksUdpIo {
}

fn poll_send_ready(&self, cx: &mut Context<'_>) -> Poll<io::Result<()>> {
let mut w = self.w.try_lock().expect("must acquire");
let mut w = self.w.lock().unwrap();
w.poll_ready_unpin(cx)
.map_err(|e| new_io_error(e.to_string()))
}
Expand All @@ -255,7 +256,7 @@ impl DatagramReceive for ShadowsocksUdpIo {
cx: &mut Context<'_>,
buf: &mut ReadBuf<'_>,
) -> Poll<io::Result<()>> {
let mut g = self.r.try_lock().expect("must acquire");
let mut g = self.r.lock().unwrap();
let (r, remained) = &mut *g;

Comment on lines +259 to 261
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::sync::Mutex::lock().unwrap() in poll_recv has the same issues as on the send side: potential blocking inside a poll function and a possible panic if the mutex is poisoned. Prefer a non-blocking lock acquisition (or redesign so locking isn’t needed) and convert lock errors into io::Error rather than panicking.

Copilot uses AI. Check for mistakes.
if !remained.is_empty() {
Expand Down
9 changes: 4 additions & 5 deletions clash-lib/src/proxy/shadowsocks/outbound/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub struct HandlerOptions {

pub struct Handler {
opts: HandlerOptions,

ctx: Arc<shadowsocks::context::Context>,
connector: tokio::sync::RwLock<Option<Arc<dyn RemoteConnector>>>,
}

Expand All @@ -65,6 +65,7 @@ impl Handler {
pub fn new(opts: HandlerOptions) -> Self {
Self {
opts,
ctx: Context::new_shared(ServerType::Local),
connector: tokio::sync::RwLock::new(None),
}
}
Expand All @@ -80,11 +81,10 @@ impl Handler {
None => s,
};

let ctx = Context::new_shared(ServerType::Local);
let cfg = self.server_config()?;

let stream = ProxyClientStream::from_stream(
ctx,
self.ctx.clone(),
stream,
&cfg,
(sess.destination.host(), sess.destination.port()),
Expand Down Expand Up @@ -198,7 +198,6 @@ impl OutboundHandler for Handler {
resolver: ThreadSafeDNSResolver,
connector: &dyn RemoteConnector,
) -> io::Result<BoxedChainedDatagram> {
let ctx = Context::new_shared(ServerType::Local);
let cfg = self.server_config()?;

let socket = connector
Expand All @@ -214,7 +213,7 @@ impl OutboundHandler for Handler {

let socket = ProxySocket::from_socket(
UdpSocketType::Client,
ctx,
self.ctx.clone(),
&cfg,
ShadowsocksUdpIo::new(socket),
);
Expand Down
Loading
Loading