Skip to content

Commit 51dc466

Browse files
wan9chiclaude
andcommitted
refactor(ipc): drop id correlation; use NativeStr/OsStr for env names
- Drop the `id: u32` field from `Request::GetEnv` and the `Response` wrapper. Each IPC connection is request-response sequential with a single in-flight reply, so correlation adds no value. `Response` + `ResponseBody` collapse into a single `GetEnvResponse { env_value }`. - Switch env-var names in requests from `&str` to `&NativeStr`, and `Handler::get_env` / `Client::get_env` to `&OsStr`. Handles non-UTF-8 env names correctly on both platforms. - `Client::get_env` returns `Arc<OsStr>` (was `OsString`); `Client::recv` is now generic over `T: SchemaRead` so future response types plug in without touching the framing layer. - `Recorder` keys `env_map` / `env_records` by `Arc<OsStr>` (was `Str`). - Windows socket name: build via `format!` directly (name always exceeds `Str` inline capacity, so the extra machinery provided no benefit). - Drop `vite_str` dep from `vite_task_server`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b792e0e commit 51dc466

6 files changed

Lines changed: 66 additions & 70 deletions

File tree

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/vite_task_client/src/lib.rs

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
use std::{
2-
ffi::{OsStr, OsString},
2+
ffi::OsStr,
33
io::{self, Read, Write},
4+
sync::Arc,
45
};
56

67
use interprocess::local_socket::{Stream, prelude::*};
78
use native_str::NativeStr;
89
use vite_path::AbsolutePath;
9-
use vite_task_ipc_shared::{IPC_ENV_NAME, Request, Response, ResponseBody};
10+
use vite_task_ipc_shared::{GetEnvResponse, IPC_ENV_NAME, Request};
11+
use wincode::{SchemaRead, config::DefaultConfig};
1012

1113
pub struct Client {
1214
stream: Stream,
13-
next_id: u32,
1415
scratch: Vec<u8>,
1516
}
1617

@@ -30,7 +31,7 @@ impl Client {
3031
for (name, value) in envs {
3132
if name.as_ref() == IPC_ENV_NAME {
3233
let stream = Stream::connect(resolve_name(value.as_ref())?)?;
33-
return Ok(Some(Self { stream, next_id: 0, scratch: Vec::new() }));
34+
return Ok(Some(Self { stream, scratch: Vec::new() }));
3435
}
3536
}
3637
Ok(None)
@@ -43,7 +44,7 @@ impl Client {
4344
/// Returns an error if the connection cannot be established.
4445
pub fn from_name(name: &OsStr) -> io::Result<Self> {
4546
let stream = Stream::connect(resolve_name(name)?)?;
46-
Ok(Self { stream, next_id: 0, scratch: Vec::new() })
47+
Ok(Self { stream, scratch: Vec::new() })
4748
}
4849

4950
/// `path` can be a file or a directory; for a directory, all files inside it are ignored.
@@ -79,20 +80,15 @@ impl Client {
7980
/// # Errors
8081
///
8182
/// Returns an error if the request or response fails, or the response id mismatches.
82-
pub fn get_env(&mut self, name: &str, tracked: bool) -> io::Result<Option<OsString>> {
83-
let id = self.next_id;
84-
self.next_id = self.next_id.wrapping_add(1);
83+
pub fn get_env(&mut self, name: &OsStr, tracked: bool) -> io::Result<Option<Arc<OsStr>>> {
84+
let name = Box::<NativeStr>::from(name);
8585

86-
self.send(&Request::GetEnv { id, name, tracked })?;
87-
self.recv()?;
86+
self.send(&Request::GetEnv { name: &name, tracked })?;
87+
let get_env_response = self.recv::<GetEnvResponse>()?;
8888

89-
let response: Response<'_> = wincode::deserialize_exact(&self.scratch)
90-
.map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?;
91-
if response.id != id {
92-
return Err(io::Error::new(io::ErrorKind::InvalidData, "response id mismatch"));
93-
}
94-
let ResponseBody::Env(value) = response.body;
95-
Ok(value.map(native_str_to_os_string))
89+
Ok(get_env_response
90+
.env_value
91+
.map(|env_value| Arc::<OsStr>::from(env_value.to_cow_os_str().as_ref())))
9692
}
9793

9894
fn send(&mut self, request: &Request<'_>) -> io::Result<()> {
@@ -106,14 +102,15 @@ impl Client {
106102
Ok(())
107103
}
108104

109-
fn recv(&mut self) -> io::Result<()> {
105+
fn recv<'a, T: SchemaRead<'a, DefaultConfig, Dst = T>>(&'a mut self) -> io::Result<T> {
110106
let mut len_bytes = [0u8; 4];
111107
self.stream.read_exact(&mut len_bytes)?;
112108
let len = u32::from_le_bytes(len_bytes) as usize;
113109
self.scratch.clear();
114110
self.scratch.resize(len, 0);
115111
self.stream.read_exact(&mut self.scratch)?;
116-
Ok(())
112+
wincode::deserialize_exact(&self.scratch)
113+
.map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))
117114
}
118115
}
119116

@@ -132,7 +129,3 @@ fn resolve_name(name: &OsStr) -> io::Result<interprocess::local_socket::Name<'_>
132129
fn path_to_native_str(path: &AbsolutePath) -> Box<NativeStr> {
133130
Box::<NativeStr>::from(path.as_path().as_os_str())
134131
}
135-
136-
fn native_str_to_os_string(ns: &NativeStr) -> OsString {
137-
ns.to_cow_os_str().into_owned()
138-
}

crates/vite_task_ipc_shared/src/lib.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,11 @@ pub const IPC_ENV_NAME: &str = "VITE_TASK_IPC_NAME";
77
pub enum Request<'a> {
88
IgnoreInput(&'a NativeStr),
99
IgnoreOutput(&'a NativeStr),
10-
GetEnv { id: u32, name: &'a str, tracked: bool },
10+
GetEnv { name: &'a NativeStr, tracked: bool },
1111
DisableCache,
1212
}
1313

1414
#[derive(Debug, SchemaWrite, SchemaRead)]
15-
pub struct Response<'a> {
16-
pub id: u32,
17-
pub body: ResponseBody<'a>,
18-
}
19-
20-
#[derive(Debug, SchemaWrite, SchemaRead)]
21-
pub enum ResponseBody<'a> {
22-
Env(Option<&'a NativeStr>),
15+
pub struct GetEnvResponse<'a> {
16+
pub env_value: Option<&'a NativeStr>,
2317
}

crates/vite_task_server/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ tokio = { workspace = true, features = ["io-util", "net", "rt", "macros"] }
1616
tokio-util = { workspace = true }
1717
tracing = { workspace = true }
1818
vite_path = { workspace = true }
19-
vite_str = { workspace = true }
2019
vite_task_ipc_shared = { workspace = true }
2120
wincode = { workspace = true, features = ["derive"] }
2221

crates/vite_task_server/src/lib.rs

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ use rustc_hash::{FxHashMap, FxHashSet};
1515
use tokio::io::{AsyncReadExt, AsyncWriteExt};
1616
use tokio_util::sync::CancellationToken;
1717
use vite_path::AbsolutePath;
18-
use vite_str::Str;
19-
use vite_task_ipc_shared::{IPC_ENV_NAME, Request, Response, ResponseBody};
18+
use vite_task_ipc_shared::{GetEnvResponse, IPC_ENV_NAME, Request};
19+
use wincode::{SchemaWrite, config::DefaultConfig};
2020

2121
pub trait Handler {
2222
fn ignore_input(&mut self, path: &Arc<AbsolutePath>);
2323
fn ignore_output(&mut self, path: &Arc<AbsolutePath>);
2424
fn disable_cache(&mut self);
25-
fn get_env(&mut self, name: &str, tracked: bool) -> Option<Arc<OsStr>>;
25+
fn get_env(&mut self, name: &OsStr, tracked: bool) -> Option<Arc<OsStr>>;
2626
}
2727

2828
/// A [`Handler`] that records every report and resolves `get_env` against
@@ -34,8 +34,8 @@ pub struct Recorder {
3434
ignored_inputs: FxHashSet<Arc<AbsolutePath>>,
3535
ignored_outputs: FxHashSet<Arc<AbsolutePath>>,
3636
cache_disabled: bool,
37-
env_records: FxHashMap<Str, EnvRecord>,
38-
env_map: FxHashMap<Str, Arc<OsStr>>,
37+
env_records: FxHashMap<Arc<OsStr>, EnvRecord>,
38+
env_map: FxHashMap<Arc<OsStr>, Arc<OsStr>>,
3939
}
4040

4141
/// A record of an env value requested via `get_env`.
@@ -54,12 +54,12 @@ pub struct Reports {
5454
pub ignored_inputs: FxHashSet<Arc<AbsolutePath>>,
5555
pub ignored_outputs: FxHashSet<Arc<AbsolutePath>>,
5656
pub cache_disabled: bool,
57-
pub env_records: FxHashMap<Str, EnvRecord>,
57+
pub env_records: FxHashMap<Arc<OsStr>, EnvRecord>,
5858
}
5959

6060
impl Recorder {
6161
#[must_use]
62-
pub fn new(env_map: FxHashMap<Str, Arc<OsStr>>) -> Self {
62+
pub fn new(env_map: FxHashMap<Arc<OsStr>, Arc<OsStr>>) -> Self {
6363
Self {
6464
ignored_inputs: FxHashSet::default(),
6565
ignored_outputs: FxHashSet::default(),
@@ -93,7 +93,7 @@ impl Handler for Recorder {
9393
self.cache_disabled = true;
9494
}
9595

96-
fn get_env(&mut self, name: &str, tracked: bool) -> Option<Arc<OsStr>> {
96+
fn get_env(&mut self, name: &OsStr, tracked: bool) -> Option<Arc<OsStr>> {
9797
if let Some(existing) = self.env_records.get_mut(name) {
9898
existing.tracked |= tracked;
9999
return existing.value.clone();
@@ -187,10 +187,15 @@ fn bind_listener() -> io::Result<(OsString, Bound)> {
187187
fn bind_listener() -> io::Result<(OsString, Bound)> {
188188
use interprocess::local_socket::{GenericNamespaced, ToNsName};
189189

190-
let name_str = vite_str::format!("vite_task_ipc_{}", uuid::Uuid::new_v4());
191-
let name = name_str.as_str().to_ns_name::<GenericNamespaced>()?;
192-
let listener = ListenerOptions::new().name(name).create_tokio()?;
193-
Ok((OsString::from(name_str.as_str()), listener))
190+
#[expect(
191+
clippy::disallowed_macros,
192+
reason = "socket name always exceeds Str inline capacity; format! is the simplest construction"
193+
)]
194+
let name = OsString::from(format!("vite_task_ipc_{}", uuid::Uuid::new_v4()));
195+
196+
let ns_name = name.as_os_str().to_ns_name::<GenericNamespaced>()?;
197+
let listener = ListenerOptions::new().name(ns_name).create_tokio()?;
198+
Ok((name, listener))
194199
}
195200

196201
#[cfg(unix)]
@@ -265,11 +270,10 @@ async fn handle_client<H: Handler>(mut stream: Stream, handler: &RefCell<H>) {
265270
}
266271
}
267272
Request::DisableCache => handler.borrow_mut().disable_cache(),
268-
Request::GetEnv { id, name, tracked } => {
269-
let value = handler.borrow_mut().get_env(name, tracked);
273+
Request::GetEnv { name, tracked } => {
274+
let value = handler.borrow_mut().get_env(name.to_cow_os_str().as_ref(), tracked);
270275
let boxed: Option<Box<NativeStr>> = value.as_deref().map(Into::into);
271-
let body = ResponseBody::Env(boxed.as_deref());
272-
let response = Response { id, body };
276+
let response = GetEnvResponse { env_value: boxed.as_deref() };
273277
if let Err(err) = write_response(&mut stream, &response).await {
274278
tracing::warn!(?err, "vite_task_server: write response failed");
275279
return;
@@ -303,7 +307,10 @@ async fn read_frame(stream: &mut Stream, buf: &mut Vec<u8>) -> io::Result<()> {
303307
Ok(())
304308
}
305309

306-
async fn write_response(stream: &mut Stream, response: &Response<'_>) -> io::Result<()> {
310+
async fn write_response<T>(stream: &mut Stream, response: &T) -> io::Result<()>
311+
where
312+
T: SchemaWrite<DefaultConfig, Src = T> + ?Sized,
313+
{
307314
let bytes = wincode::serialize(response)
308315
.map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?;
309316
let len = u32::try_from(bytes.len())

crates/vite_task_server/tests/integration.rs

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,21 @@ use std::{
77
use rustc_hash::FxHashMap;
88
use tokio::runtime::Builder;
99
use vite_path::AbsolutePathBuf;
10-
use vite_str::Str;
1110
use vite_task_client::Client;
1211
use vite_task_server::{Recorder, Reports, ServerHandle, serve};
1312

1413
fn abs(path: &str) -> AbsolutePathBuf {
1514
AbsolutePathBuf::new(path.into()).expect("absolute path literal")
1615
}
1716

18-
fn env_map(pairs: &[(&str, &str)]) -> FxHashMap<Str, Arc<OsStr>> {
19-
pairs.iter().map(|(k, v)| (Str::from(*k), Arc::<OsStr>::from(OsStr::new(v)))).collect()
17+
fn env_map(pairs: &[(&str, &str)]) -> FxHashMap<Arc<OsStr>, Arc<OsStr>> {
18+
pairs
19+
.iter()
20+
.map(|(k, v)| (Arc::<OsStr>::from(OsStr::new(k)), Arc::<OsStr>::from(OsStr::new(v))))
21+
.collect()
2022
}
2123

22-
fn run_with_server<F>(envs: FxHashMap<Str, Arc<OsStr>>, client_work: F) -> Reports
24+
fn run_with_server<F>(envs: FxHashMap<Arc<OsStr>, Arc<OsStr>>, client_work: F) -> Reports
2325
where
2426
F: FnOnce(OsString) + Send + 'static,
2527
{
@@ -62,17 +64,17 @@ fn single_client_fire_and_forget() {
6264
fn get_env_found_and_not_found() {
6365
let reports = run_with_server(env_map(&[("NODE_ENV", "production")]), |name| {
6466
let mut client = Client::from_name(&name).expect("connect");
65-
let present = client.get_env("NODE_ENV", true).unwrap();
67+
let present = client.get_env(OsStr::new("NODE_ENV"), true).unwrap();
6668
assert_eq!(present.as_deref(), Some(OsStr::new("production")));
67-
let missing = client.get_env("MISSING", false).unwrap();
69+
let missing = client.get_env(OsStr::new("MISSING"), false).unwrap();
6870
assert!(missing.is_none());
6971
});
7072

71-
let node = reports.env_records.get("NODE_ENV").expect("NODE_ENV recorded");
73+
let node = reports.env_records.get(OsStr::new("NODE_ENV")).expect("NODE_ENV recorded");
7274
assert!(node.tracked);
7375
assert_eq!(node.value.as_deref(), Some(OsStr::new("production")));
7476

75-
let missing = reports.env_records.get("MISSING").expect("MISSING recorded");
77+
let missing = reports.env_records.get(OsStr::new("MISSING")).expect("MISSING recorded");
7678
assert!(!missing.tracked);
7779
assert!(missing.value.is_none());
7880
}
@@ -81,29 +83,31 @@ fn get_env_found_and_not_found() {
8183
fn get_env_tracked_upgrade_is_monotonic() {
8284
let reports = run_with_server(env_map(&[("NODE_ENV", "production")]), |name| {
8385
let mut client = Client::from_name(&name).expect("connect");
84-
let a = client.get_env("NODE_ENV", false).unwrap();
85-
let b = client.get_env("NODE_ENV", true).unwrap();
86-
let c = client.get_env("NODE_ENV", false).unwrap();
86+
let a = client.get_env(OsStr::new("NODE_ENV"), false).unwrap();
87+
let b = client.get_env(OsStr::new("NODE_ENV"), true).unwrap();
88+
let c = client.get_env(OsStr::new("NODE_ENV"), false).unwrap();
8789
for v in [a, b, c] {
8890
assert_eq!(v.as_deref(), Some(OsStr::new("production")));
8991
}
9092
});
9193

92-
let node = reports.env_records.get("NODE_ENV").expect("recorded");
94+
let node = reports.env_records.get(OsStr::new("NODE_ENV")).expect("recorded");
9395
assert!(node.tracked, "tracked must remain true once set");
9496
}
9597

9698
#[test]
9799
fn concurrent_clients() {
98-
let reports = run_with_server(env_map(&[("SHARED", "value")]), |name| {
99-
let threads: Vec<_> = (0..4)
100-
.map(|i| {
100+
let paths = ["/tmp/worker_0", "/tmp/worker_1", "/tmp/worker_2", "/tmp/worker_3"];
101+
let reports = run_with_server(env_map(&[("SHARED", "value")]), move |name| {
102+
let threads: Vec<_> = paths
103+
.iter()
104+
.map(|path| {
101105
let name = name.clone();
106+
let path = *path;
102107
thread::spawn(move || {
103108
let mut client = Client::from_name(&name).expect("connect");
104-
let path = vite_str::format!("/tmp/worker_{i}");
105-
client.ignore_input(abs(path.as_str()).as_absolute_path()).unwrap();
106-
let value = client.get_env("SHARED", true).unwrap();
109+
client.ignore_input(abs(path).as_absolute_path()).unwrap();
110+
let value = client.get_env(OsStr::new("SHARED"), true).unwrap();
107111
assert_eq!(value.as_deref(), Some(OsStr::new("value")));
108112
})
109113
})
@@ -114,7 +118,7 @@ fn concurrent_clients() {
114118
});
115119

116120
assert_eq!(reports.ignored_inputs.len(), 4);
117-
let shared = reports.env_records.get("SHARED").expect("recorded");
121+
let shared = reports.env_records.get(OsStr::new("SHARED")).expect("recorded");
118122
assert!(shared.tracked);
119123
assert_eq!(shared.value.as_deref(), Some(OsStr::new("value")));
120124
}

0 commit comments

Comments
 (0)