From 7c1e22f837494896f90bd7e13765bff31619ce6f Mon Sep 17 00:00:00 2001 From: WorkShop1 Date: Sat, 26 Sep 2026 20:55:17 +0500 Subject: [PATCH 1/2] transport: probe Windows named pipes without a Tokio reactor `is_socket_path` and `remove_socket` are synchronous by contract, but both opened the pipe with `ClientOptions::new().open()`. Tokio's named-pipe client registers the pipe handle with the ambient reactor, so calling either from a thread that has no runtime context panics: ``` thread '' panicked at tokio-1.49.0/src/net/windows/named_pipe.rs:1005 there is no reactor running, must be called from the context of a Tokio 1.x runtime ``` Jcode Desktop drives the SDK from plain `std::thread`s, so its bridge processes take this path and abort. The panic happens on the `open` call itself, before any success or failure is known, so it is not specific to a missing endpoint. Both probes now go through a small `probe_pipe` helper built on `std::fs::OpenOptions`, the same approach `SyncStream::connect` already uses. `ERROR_PIPE_BUSY` still maps to "a server is listening", so the busy-pipe behaviour is unchanged. Test: `sync_pipe_probes_do_not_panic_without_a_tokio_reactor` binds a real listener inside a `#[tokio::test]` runtime, then probes it from a `std::thread` that deliberately has no runtime. That split is the situation in the bug: the server owns a runtime, the synchronous probes do not. The test is load-bearing, verified rather than assumed. Two things had to be got right for it to mean anything: - the pipe must actually exist. An earlier version probed a path that was never bound, and it passed with and without this fix, because the open fails before tokio reaches the reactor registration; - the listener must be bound under a runtime. An earlier version used a plain `#[test]`, which then panicked inside `Listener::bind` instead. With `probe_pipe` reverted to `ClientOptions::open`, the spawned thread panics with the message above and this test fails; the other six tests in the crate pass in both states, so it isolates this defect. --- crates/jcode-transport/src/windows.rs | 61 ++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/crates/jcode-transport/src/windows.rs b/crates/jcode-transport/src/windows.rs index 37df039456..051f0c2146 100644 --- a/crates/jcode-transport/src/windows.rs +++ b/crates/jcode-transport/src/windows.rs @@ -340,7 +340,7 @@ impl io::Write for SyncStream { pub fn is_socket_path(path: &Path) -> bool { let pipe_name = path_to_pipe_name(path); - match ClientOptions::new().open(&pipe_name) { + match probe_pipe(&pipe_name) { Ok(_) => true, Err(error) if error.raw_os_error() @@ -357,7 +357,7 @@ pub fn is_socket_path(path: &Path) -> bool { pub fn remove_socket(path: &Path) { let pipe_name = path_to_pipe_name(path); - if ClientOptions::new().open(&pipe_name).is_ok() { + if probe_pipe(&pipe_name).is_ok() { eprintln!( "[windows] Named pipe {} still open, will be replaced by new server", pipe_name @@ -365,6 +365,22 @@ pub fn remove_socket(path: &Path) { } } +/// Probe whether a named pipe is currently reachable, without a tokio reactor. +/// +/// `ClientOptions::open` registers the pipe handle with the ambient tokio +/// reactor, so it panics with "there is no reactor running" when called from a +/// thread that has no runtime context. The two probes above are synchronous by +/// contract and are reachable from plain `std::thread` callers, so they must +/// not touch the tokio client API. `SyncStream::connect` already opens pipes +/// with `std::fs`, which is the same approach. +fn probe_pipe(pipe_name: &str) -> io::Result<()> { + std::fs::OpenOptions::new() + .read(true) + .write(true) + .open(pipe_name) + .map(|_handle| ()) +} + pub fn stream_pair() -> io::Result<(Stream, Stream)> { Stream::pair() } @@ -378,6 +394,47 @@ mod tests { static BUSY_PIPE_TEST_COUNTER: AtomicU64 = AtomicU64::new(0); + /// Jcode Desktop drives the SDK from plain `std::thread`s, where tokio's + /// named-pipe client panics with "there is no reactor running". The two + /// synchronous probes must therefore stay off the tokio client API. + /// + /// The pipe has to be a real, bound one: an absent pipe makes the open fail + /// before tokio reaches the reactor registration, so probing a missing + /// endpoint would pass with or without the fix and prove nothing. + /// + /// The test itself is a `#[tokio::test]` so that `Listener::bind` has a + /// runtime, while the `std::thread` doing the probe deliberately has none. + /// That split is the situation this bug is about: the server owns a runtime, + /// the synchronous probes are called from plain threads that do not. + #[tokio::test] + async fn sync_pipe_probes_do_not_panic_without_a_tokio_reactor() { + let path = std::env::temp_dir().join(format!( + "jcode-plain-thread-probe-{}-{}.sock", + std::process::id(), + BUSY_PIPE_TEST_COUNTER.fetch_add(1, Ordering::Relaxed) + )); + let _listener = Listener::bind(&path).expect("bind named pipe"); + + let probe = path.clone(); + let joined = std::thread::spawn(move || { + let reachable = is_socket_path(&probe); + remove_socket(&probe); + reachable + }) + .join(); + + assert!( + joined.is_ok(), + "probing a live pipe on a plain thread panicked: {:?}", + joined.err() + ); + assert_eq!( + joined.expect("thread joined"), + true, + "a bound named pipe must be reported as a live endpoint" + ); + } + /// The TypeScript SDK derives this name independently, so the two must /// agree exactly or a Windows client dials a pipe nobody is listening on. /// Pinning literal values here gives that duplicate a fixed contract to From 6c83e653b85d04e7098e60438ccbf3f4ec3d99e0 Mon Sep 17 00:00:00 2001 From: WorkShop1 Date: Sat, 26 Sep 2026 21:15:51 +0500 Subject: [PATCH 2/2] transport: probe remove_socket on its own bound pipe Reported as P2 on #1525: the regression test called `is_socket_path` and then `remove_socket` on the same single-instance listener. Probing consumes the only available instance of a Windows named pipe, so the second open can return ERROR_PIPE_BUSY and return an error without ever reaching the code under test. A reverted `ClientOptions::open` in `remove_socket` could therefore have slipped through while the test still passed. The test now binds a second, separate pipe for `remove_socket`, so each probe hits a freshly available instance and each is independently covered. Load-bearing proof repeated after the change, since the previous proof was for the previous version of the test: with `probe_pipe` reverted to `ClientOptions::open`, `sync_pipe_probes_do_not_panic_without_a_tokio_reactor` is the only one of the seven that fails, and it fails with the reactor panic in the spawned thread. --- crates/jcode-transport/src/windows.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/jcode-transport/src/windows.rs b/crates/jcode-transport/src/windows.rs index 051f0c2146..fe41666373 100644 --- a/crates/jcode-transport/src/windows.rs +++ b/crates/jcode-transport/src/windows.rs @@ -415,10 +415,22 @@ mod tests { )); let _listener = Listener::bind(&path).expect("bind named pipe"); + // A second, separately bound pipe for `remove_socket`. Probing consumes + // the only available instance of a pipe, so reusing `path` would let the + // second probe return ERROR_PIPE_BUSY and never reach the code under + // test, which is exactly the coverage a single shared pipe left out. + let remove_path = std::env::temp_dir().join(format!( + "jcode-plain-thread-remove-{}-{}.sock", + std::process::id(), + BUSY_PIPE_TEST_COUNTER.fetch_add(1, Ordering::Relaxed) + )); + let _remove_listener = Listener::bind(&remove_path).expect("bind second named pipe"); + let probe = path.clone(); + let remove_probe = remove_path.clone(); let joined = std::thread::spawn(move || { let reachable = is_socket_path(&probe); - remove_socket(&probe); + remove_socket(&remove_probe); reachable }) .join();