fix: reduce detach latency and stabilize detach path by tobwen · Pull Request #324 · shell-pool/shpool

tobwen · 2026-03-02T18:40:36Z

Summary

Fixes #323, detach latency by making detach path responsive with faster polling and adaptive backoff in client protocol loop.

Changes

Reduced JOIN_POLL_DURATION from 100ms to 10ms in libshpool/src/consts.rs for faster thread join polling
Reduced SHELL_TO_CLIENT_POLL_MS from 100 to 10 in libshpool/src/daemon/shell.rs for faster detach/reattach detection
Added adaptive backoff with exponential step (1ms initial, capped at 25ms) in client protocol loop
Added fast path for TTY detach (10ms) vs. slow path fallback (300ms max)
Exit client->shell thread immediately after detach action in libshpool/src/daemon/shell.rs
Refactored polling loop in libshpool/src/protocol.rs to check stop flag during sleep intervals

Behavior

Before this fix

Detach was slow (100 to 300ms polling intervals)
Threads could stay blocked after detach waiting on poll cycles
Users experienced noticeable delay when detaching

After this fix

Responsive detach with 10ms fast-path for TTY sessions
Immediate exit after detach (no more waiting for poll cycles)
Adaptive backoff ensures compatibility with existing shutdown paths
CPU usage controlled through bounded backoff (max 25ms step)

Rationale

The previous polling intervals (100ms) were too conservative, causing noticeable latency in the detach operation. By reducing polling to 10ms and implementing adaptive backoff, detach becomes responsive while maintaining compatibility with existing shutdown flows and avoiding busy waits.

Tests

No tests have been added or modified.

tobwen · 2026-03-03T16:30:21Z

Oh come on - formatting of a comment? Really? 😄 I'll lookup the correct formatting rules later.

ethanpailes

A good start. I left some feedback on things we can to do reuse code a bit.

ethanpailes · 2026-03-03T22:29:08Z

libshpool/src/daemon/shell.rs

                    }

-                    thread::sleep(consts::HEARTBEAT_DURATION);
+                    let mut slept = time::Duration::ZERO;


Let's bundle this logic up into a helper function that takes a stop predicate function (in this case it would be parameterized with a closure that checks the stop atomic), a poll strategy enum, a sleep duration. It seems like a generically useful pattern. We could put it in libshpool/src/common.rs

The poll strategy enum can have one option for uniform polling with a given duration (which we would use here) and another for exponential backoff with the usual exponential backoff params (initial poll dur, backoff factor, max poll dur).

we could call it sleep_unless or something like that.

ethanpailes · 2026-03-03T22:31:37Z

libshpool/src/protocol.rs

-                        if sock_to_stdout_h.is_finished() {
-                            info!("recheck: sock->stdout thread done");
-                            nfinished_threads += 1;
+                        // Fast-path: when server->client already ended (detach/disconnect),


Lets keep the thread names consistant and call this sock->stdout

ethanpailes · 2026-03-03T22:36:44Z

libshpool/src/protocol.rs

+                        let mut stdin_done = stdin_to_sock_h.is_finished();
+                        let mut stdout_done = sock_to_stdout_h.is_finished();
+
+                        let stdin_is_tty = isatty(io::stdin()).unwrap_or(false);


I generally like to avoid isatty when possible since it makes it harder to predict how a tool will work when running under a script. Sometimes it is worthwhile, but in this case I don't think it is worth having divergant behavior. I don't see a reason we would need to wait around longer in a script context, so let's just always use the short timeout if the daemon hangs up on us. We should re-name the constant to avoid mentioning TTY when we do this.

ethanpailes · 2026-03-03T22:39:09Z

libshpool/src/daemon/shell.rs

-                    thread::sleep(consts::HEARTBEAT_DURATION);
+                    let mut slept = time::Duration::ZERO;
+                    let sleep_step = consts::JOIN_POLL_DURATION;
+                    while slept < consts::HEARTBEAT_DURATION {


Rather than incrementing a counter of total duration slept, these sorts of loops should instead set a specific deadline in the future and always compare the current time against that. This avoid lock drift because thread::sleep(d) does not alwasy take exactly d. These little mismatches can add up over time. Not really a huge deal, but it doesn't hurt to be as precice as possible.

ethanpailes · 2026-03-03T22:40:57Z

libshpool/src/daemon/shell.rs

+                        }
+
+                        let remaining = consts::HEARTBEAT_DURATION - slept;
+                        let step = if remaining < sleep_step { remaining } else { sleep_step };


lets use min the way you do below. Also, we probably don't need an explicit named variable for remaining, we can just inline it into the min params for brevity.

ethanpailes · 2026-03-03T22:43:37Z

libshpool/src/protocol.rs

+                            MAX_DETACH_WAIT_DUR
+                        };
+
+                        loop {


This loop can switch to using the helper I suggested above, with its stop predicate computing nfinished_threads and checking if the count is >= 2.

tobwen · 2026-03-03T23:52:16Z

Phew, that was way more work than I expected. Hopefully I haven't overlooked anything or made it more complicated than planned. And - of course - I missed one 😅

ethanpailes

Nearly there, just one last nit about the new sleep API

ethanpailes · 2026-03-04T15:39:41Z

libshpool/src/common.rs

+    /// Poll at a fixed interval.
+    Uniform { interval: time::Duration },
+    /// Poll with exponential backoff up to a maximum interval.
+    Backoff { initial_interval: time::Duration, factor: u32, max_interval: time::Duration },


Let's use a float for the factor. The most common backoff factors are 2 and 1.5, and if we use an int we can't handle 1.5.

ethanpailes

I switched factor to a float.

tobwen · 2026-03-10T23:26:51Z

Sorry, I didn’t get around to it and didn’t want to submit something half-baked. It’s great that you made the changes! Thank you very much.

tobwen mentioned this pull request Mar 2, 2026

[HAS FIX] Improve detach latency and responsivenes #323

Closed

ethanpailes force-pushed the fix/detach-latency branch from 757b1bd to 994ea81 Compare March 3, 2026 16:14

tobwen force-pushed the fix/detach-latency branch 2 times, most recently from 1013438 to b689d04 Compare March 3, 2026 19:54

tobwen added 3 commits March 3, 2026 21:14

fix: reduce detach latency and stabilize detach path

df8d0b3

fix: reduce detach latency and stabilize detach path

b92fb5b

chore: fix rustfmt nightly comment wrapping

af4a3cd

tobwen force-pushed the fix/detach-latency branch from b689d04 to af4a3cd Compare March 3, 2026 21:14

ethanpailes reviewed Mar 3, 2026

View reviewed changes

tobwen added 3 commits March 3, 2026 23:37

refactor: add shared sleep helper for stop-aware polling

621c94d

refactor: simplify detach wait logic with shared polling

af727f4

docs: clarify generalized detach fast-path behavior

7a7b77e

docs: keep sock->stdout naming consistent

5daea97

ethanpailes reviewed Mar 4, 2026

View reviewed changes

switch factor to float

26ecd15

ethanpailes approved these changes Mar 10, 2026

View reviewed changes

ethanpailes merged commit e9f8a57 into shell-pool:master Mar 10, 2026
7 checks passed

release-plz-for-shpool bot mentioned this pull request Mar 3, 2026

chore: release #319

Merged

Conversation

tobwen commented Mar 2, 2026

Summary

Changes

Behavior

Before this fix

After this fix

Rationale

Tests

Uh oh!

tobwen commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ethanpailes left a comment

Choose a reason for hiding this comment

Uh oh!

ethanpailes Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ethanpailes Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ethanpailes Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ethanpailes Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ethanpailes Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ethanpailes Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

tobwen commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ethanpailes left a comment

Choose a reason for hiding this comment

Uh oh!

ethanpailes Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

ethanpailes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tobwen commented Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tobwen commented Mar 3, 2026 •

edited

Loading

tobwen commented Mar 3, 2026 •

edited

Loading