Skip to content
Merged
8 changes: 8 additions & 0 deletions src/http/AsyncHTTP.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ fn make_client<'a>(
prev_redirect: Vec::new(),
progress_node: None,
flags: Flags::default(),
idle_timeout_seconds: None,
state: InternalState::default(),
tls_props: None,
custom_ssl_ctx: None,
Expand Down Expand Up @@ -267,6 +268,9 @@ pub struct Options<'a> {
pub signals: Option<Signals>,
pub unix_socket_path: Option<ZigStringSlice>,
pub disable_timeout: Option<bool>,
/// Per-request idle timeout override in seconds; see
/// `HTTPClient::idle_timeout_seconds`.
pub idle_timeout_seconds: Option<core::ffi::c_uint>,
pub verbose: Option<HTTPVerboseLevel>,
pub disable_keepalive: Option<bool>,
pub disable_decompression: Option<bool>,
Expand Down Expand Up @@ -516,6 +520,10 @@ impl<'a> AsyncHTTP<'a> {
if let Some(val) = options.disable_timeout {
this.client.flags.disable_timeout = val;
}
if let Some(val) = options.idle_timeout_seconds {
this.client.idle_timeout_seconds =
Some(crate::normalize_idle_timeout_seconds(val.into()));
}
if let Some(val) = options.verbose {
this.client.verbose = val;
}
Expand Down
27 changes: 9 additions & 18 deletions src/http/HTTPThread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1257,25 +1257,16 @@ mod _event_loop_draft {
pub(super) fn on_start(opts: InitOpts) {
Output::Source::configure_named_thread(bun_core::zstr!("HTTP Client"));

// uSockets' long-timeout counter is `% 240` minutes (see
// `us_socket_long_timeout` in packages/bun-usockets/src/socket.c), so
// values above 239 min wrap around and fire early. Clamp here — it's the
// only assignment — so the underlying timer can't wrap, and round values
// above 240s up to a whole minute so `socket.set_timeout`'s floor-to-
// minute long-timer path never yields a timeout *shorter* than requested.
// Normalising once here keeps the h1 (`HTTPClient::set_timeout`) and h2
// (`ClientSession::rearm_timeout`) paths identical without duplicating the
// math at each call site.
let raw: u64 = bun_core::env_var::BUN_CONFIG_HTTP_IDLE_TIMEOUT
.get()
.unwrap_or(300)
.min(239 * 60);
// Normalising once here (see `normalize_idle_timeout_seconds`) keeps
// the h1 (`HTTPClient::set_timeout`) and h2
// (`ClientSession::rearm_timeout`) paths identical without duplicating
// the math at each call site.
crate::IDLE_TIMEOUT_SECONDS.store(
(if raw > 240 {
raw.div_ceil(60) * 60
} else {
raw
}) as core::ffi::c_uint,
crate::normalize_idle_timeout_seconds(
bun_core::env_var::BUN_CONFIG_HTTP_IDLE_TIMEOUT
.get()
.unwrap_or(300),
),
core::sync::atomic::Ordering::Relaxed,
);

Expand Down
31 changes: 12 additions & 19 deletions src/http/h2_client/ClientSession.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,28 +517,21 @@
/// would win and a `{timeout:false}` long-poll could be killed by a
/// sibling re-arming, or strip the safety net from one that wants it),
/// so the session disarms only when *every* attached client opted out.
fn rearm_timeout(&mut self) {
let want = 'blk: {
for &s in self.streams.values() {
if let Some(c) = stream_ref(s).client_ref() {
if !c.flags.disable_timeout {
break 'blk true;
}
}
}
for &c in &self.pending_attach {
if !pending_client_mut(c).flags.disable_timeout {
break 'blk true;
}
// The socket is shared by every stream on the session, so arm the
// longest effective idle timeout among them (0 = every client
// disabled the timer, or none are attached).
let mut want: core::ffi::c_uint = 0;
for &s in self.streams.values() {
if let Some(c) = stream_ref(s).client_ref() {
want = want.max(c.effective_idle_timeout_seconds());
}
false
};
self.socket.set_timeout(if want {
crate::idle_timeout_seconds()
} else {
0
});
}
for &c in &self.pending_attach {
want = want.max(pending_client_mut(c).effective_idle_timeout_seconds());
}
self.socket.set_timeout(want);
}

Check failure on line 534 in src/http/h2_client/ClientSession.rs

View check run for this annotation

Claude / Claude Code Review

h2 rearm_timeout: {timeout:false} stream can be killed by a sibling's short explicit timeout

A `{timeout: false}` request coalesced onto an h2 session can now be killed by a sibling's short explicit `timeout`: `effective_idle_timeout_seconds()` returns 0 for `disable_timeout`, so it contributes nothing to the `max()` and a `{timeout: 5000}` sibling arms the shared socket at 5s — when it fires, `on_long_timeout` → `session.on_close` fails **every** stream with no per-stream `disable_timeout` re-check. Before this PR the same session armed the global 300s default, so the `{timeout: false}
Comment thread
robobun marked this conversation as resolved.

/// HTTP-thread wake-up from `scheduleResponseBodyDrain`: JS just enabled
/// `response_body_streaming`, so flush any body bytes that arrived between
Expand Down
49 changes: 37 additions & 12 deletions src/http/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,20 @@ pub fn idle_timeout_seconds() -> c_uint {
IDLE_TIMEOUT_SECONDS.load(Ordering::Relaxed)
}

/// Normalise an idle timeout (seconds) for uSockets' timers: the long-timeout
/// counter wraps `% 240` minutes, so clamp to 239 min, and values above 240s
/// are served by the minute-granularity long timer, so round them up to a
/// whole minute so the floor-to-minute path never fires *earlier* than asked.
#[inline]
pub fn normalize_idle_timeout_seconds(raw: u64) -> c_uint {
Comment thread
cirospaciari marked this conversation as resolved.
let raw = raw.min(239 * 60);
(if raw > 240 {
raw.div_ceil(60) * 60
} else {
raw
}) as c_uint
}

pub const END_OF_CHUNKED_HTTP1_1_ENCODING_RESPONSE_BODY: &[u8] = b"0\r\n\r\n";

/// HTTP-thread-only scratch buffer for building NUL-terminated hostnames.
Expand Down Expand Up @@ -623,6 +637,11 @@ pub struct HTTPClient<'a> {

pub flags: Flags,

/// Per-request override of the global [`IDLE_TIMEOUT_SECONDS`], set from
/// `fetch(url, { timeout: <ms> })`. Already normalised (see
/// [`normalize_idle_timeout_seconds`]). `None` = use the global default.
pub idle_timeout_seconds: Option<c_uint>,

pub state: InternalState<'a>,
pub tls_props: Option<ssl_config::SharedPtr>,
/// The custom SSL context used for this request (None = default context).
Expand Down Expand Up @@ -3893,19 +3912,25 @@ impl<'a> HTTPClient<'a> {
}
}

pub fn set_timeout<S: SocketTimeout>(&self, socket: &S) {
// Duration comes from `IDLE_TIMEOUT_SECONDS` (tunable via
// `BUN_CONFIG_HTTP_IDLE_TIMEOUT`, set low in tests) and is normalised once
// in `HTTPThread::on_start` — clamped to the uSockets long-timer bound and
// rounded up to a whole minute above 240s — so this is a plain
// pass-through. `socket.set_timeout` picks the short-tick timer for values
// ≤ 240s and the minute-granularity long timer above that, so the default
// 300s maps to the same 5-minute long timer as before.
if self.flags.disable_timeout || idle_timeout_seconds() == 0 {
socket.set_timeout(0);
return;
/// The idle timeout to arm for this request, in seconds (0 = disabled):
/// the per-request `fetch({ timeout })` override when present, otherwise
/// the global `BUN_CONFIG_HTTP_IDLE_TIMEOUT` default. Both are already
/// normalised (see [`normalize_idle_timeout_seconds`]).
#[inline]
pub fn effective_idle_timeout_seconds(&self) -> c_uint {
if self.flags.disable_timeout {
return 0;
}
socket.set_timeout(idle_timeout_seconds());
self.idle_timeout_seconds
.unwrap_or_else(idle_timeout_seconds)
}

pub fn set_timeout<S: SocketTimeout>(&self, socket: &S) {
// Values are pre-normalised (global: `HTTPThread::on_start`;
// per-request: `AsyncHTTP::init`) so this is a plain pass-through.
// `socket.set_timeout` picks the short-tick timer for values ≤ 240s
// and the minute-granularity long timer above that.
socket.set_timeout(self.effective_idle_timeout_seconds());
}

fn maybe_pause_receive<const IS_SSL: bool>(&mut self, socket: HttpSocket<IS_SSL>) {
Expand Down
11 changes: 11 additions & 0 deletions src/runtime/webcore/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
let first_arg = args.next_eat().unwrap();

let mut disable_timeout = false;
let mut idle_timeout_seconds: Option<core::ffi::c_uint> = None;
let mut disable_keepalive = false;
let mut disable_decompression = false;
let mut compress: Option<compress_body::CompressOption> = None;
Expand Down Expand Up @@ -860,6 +861,15 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
if timeout_value.is_boolean() {
break 'extract_disable_timeout !timeout_value.as_boolean();
} else if timeout_value.is_number() {
// A finite positive `timeout` (in ms) also governs the
// socket idle deadline, overriding the global
// `BUN_CONFIG_HTTP_IDLE_TIMEOUT` default.
let ms = timeout_value.as_number();
if ms.is_finite() && ms > 0.0 {
idle_timeout_seconds =
Some((ms / 1000.0).ceil().min(core::ffi::c_uint::MAX as f64)
as core::ffi::c_uint);
}
break 'extract_disable_timeout timeout_value.to_int32() == 0;
Comment thread
robobun marked this conversation as resolved.
Outdated
}
}
Expand Down Expand Up @@ -2021,6 +2031,7 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
body,
disable_keepalive,
disable_timeout,
idle_timeout_seconds,
disable_decompression,
max_redirects,
reject_unauthorized,
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/webcore/fetch/FetchTasklet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1996,6 +1996,7 @@ impl FetchTasklet {
signals: Some(fetch_tasklet.signals),
unix_socket_path: Some(fetch_options.unix_socket_path),
disable_timeout: Some(fetch_options.disable_timeout),
idle_timeout_seconds: fetch_options.idle_timeout_seconds,
disable_keepalive: Some(fetch_options.disable_keepalive),
disable_decompression: Some(fetch_options.disable_decompression),
max_redirects: fetch_options.max_redirects,
Expand Down Expand Up @@ -2517,6 +2518,8 @@ pub struct FetchOptions {
pub headers: Headers,
pub body: HTTPRequestBody,
pub disable_timeout: bool,
/// Per-request idle-timeout override, from `fetch(url, { timeout: <ms> })`.
pub idle_timeout_seconds: Option<core::ffi::c_uint>,
pub disable_keepalive: bool,
pub disable_decompression: bool,
pub max_redirects: Option<u8>,
Expand Down Expand Up @@ -2553,6 +2556,7 @@ impl Default for FetchOptions {
headers: Headers::default(),
body: HTTPRequestBody::default(),
disable_timeout: false,
idle_timeout_seconds: None,
disable_keepalive: false,
disable_decompression: false,
max_redirects: None,
Expand Down
54 changes: 54 additions & 0 deletions test/js/web/fetch/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2883,3 +2883,57 @@ it("does not reuse a keep-alive connection whose response carried more bytes tha
server.close();
}
});

// https://github.com/oven-sh/bun/issues/16682
it("an explicit numeric `timeout` extends the socket idle deadline past the default", async () => {
// The child runs with a 1s idle default (BUN_CONFIG_HTTP_IDLE_TIMEOUT=1) and
// talks to an in-process server whose handler holds every request idle for
// 10s (longer than the worst-case firing window of the 1s idle timer, which
// is swept on uSockets' 4s tick) before responding.
//
// - `timeout: 60_000` must override the 1s idle default and resolve.
// - `timeout: 0` must keep meaning "no timeout" and resolve.
// - no `timeout` at all must still hit the 1s idle default (control that
// proves the env override and the stall are both real).
const script = /* js */ `
const HOLD_MS = 10_000;
using server = Bun.serve({
port: 0,
// Disable Bun.serve's own request idle timeout; only the client-side
// idle timer under test may abort anything here.
idleTimeout: 0,
async fetch(req) {
const arrived = Date.now();
// Hold the connection idle (no bytes in either direction) until the
// hold window has really elapsed on the server's clock.
while (Date.now() - arrived < HOLD_MS) {
await Bun.sleep(HOLD_MS - (Date.now() - arrived));
}
return new Response("hello");
},
});
const get = init => fetch(server.url, init).then(r => r.text(), e => "ERR:" + (e?.code ?? e?.name ?? e));
const [withTimeout, withZero, withDefault] = await Promise.all([
get({ timeout: 60_000 }),
get({ timeout: 0 }),
get(undefined),
]);
console.log(JSON.stringify({ withTimeout, withZero, withDefault }));
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", script],
env: { ...bunEnv, BUN_CONFIG_HTTP_IDLE_TIMEOUT: "1" },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
const out = JSON.parse(stdout.trim().split("\n").pop()!) as Record<string, string>;
expect({ withTimeout: out.withTimeout, withZero: out.withZero }).toEqual({
withTimeout: "hello",
withZero: "hello",
});
// Control: without an explicit `timeout`, the 1s idle default still aborts
// the stalled request.
expect(out.withDefault).toStartWith("ERR:");
expect(exitCode).toBe(0);
}, 60_000);
Comment thread
robobun marked this conversation as resolved.
Loading