Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
9 changes: 9 additions & 0 deletions src/js/internal/timers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ const NumberIsFinite = Number.isFinite;

const TIMEOUT_MAX = 2 ** 31 - 1;

/**
* Monotonic milliseconds for deadlines the runtime itself measures. Not
* `Date.now()` / `performance.now()`: bun:test's `setSystemTime()` and
* `useFakeTimers()` override both inside the engine (capturing the function
* does not help), which would freeze or skew an internal deadline.
*/
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
const monotonicNowMs = $newRustFunction("runtime/timer/Timer.rs", "internal_bindings.monotonicNowMs", 0);

function getTimerDuration(msecs, name) {
validateNumber(msecs, name);
if (msecs < 0 || !NumberIsFinite(msecs)) {
Expand All @@ -29,4 +37,5 @@ export default {
// tests that inspect socket[kTimeout].
kTimeout: Symbol.for("::buntimeout::"),
getTimerDuration,
monotonicNowMs,
};
11 changes: 7 additions & 4 deletions src/js/node/_http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const {
validateMsecs,
} = require("internal/http");
const { FakeSocket } = require("internal/http/FakeSocket");
const { monotonicNowMs } = require("internal/timers");
const NumberIsNaN = Number.isNaN;

const { format } = require("internal/util/inspect");
Expand Down Expand Up @@ -120,7 +121,6 @@ const kEmptyBuffer = Buffer.alloc(0);
const ObjectKeys = Object.keys;
const MathMin = Math.min;
const MathFloor = Math.floor;
const DateNow = Date.now;

let cluster;

Expand Down Expand Up @@ -1419,7 +1419,10 @@ const kKeepAliveTimeoutSet = Symbol("keepAliveTimeoutSet");
// When the keep-alive idle period on a connection started (the last response
// finish). onResponseFinishHandleSocket records this instead of rescheduling
// the socket timer on every response; onSocketTimeoutTimerExpired reads it to
// grant the remaining idle budget when the timer actually fires.
// grant the remaining idle budget when the timer actually fires. Taken with
// monotonicNowMs(), not Date.now(): bun:test's setSystemTime() / fake timers
// move Date.now(), which would expire the connection early or re-arm it for
// however far the clock was moved.
Comment thread
robobun marked this conversation as resolved.
Outdated
const kKeepAliveIdleStart = Symbol("keepAliveIdleStart");
// HTTP/1.1 pipelining (responses queued behind an in-flight response):
// - on the socket: array of queued ServerResponses, in arrival order
Expand Down Expand Up @@ -1518,7 +1521,7 @@ function onSocketTimeoutTimerExpired(socket) {
const idleStart = socket[kKeepAliveIdleStart];
if (idleStart !== undefined && socket[kKeepAliveTimeoutSet]) {
socket[kKeepAliveIdleStart] = undefined;
const remaining = socket.timeout - (DateNow() - idleStart);
const remaining = socket.timeout - (monotonicNowMs() - idleStart);
if (remaining > 0) {
const existingTimer = socket[kSocketTimeoutTimer];
if (existingTimer !== undefined) clearTimeout(existingTimer);
Expand Down Expand Up @@ -2519,7 +2522,7 @@ function onResponseFinishHandleSocket(server, socket, res) {
} else {
socket.setTimeout(total);
}
socket[kKeepAliveIdleStart] = DateNow();
socket[kKeepAliveIdleStart] = monotonicNowMs();
socket[kKeepAliveTimeoutSet] = true;
}
}
Expand Down
14 changes: 14 additions & 0 deletions src/runtime/timer/Timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -596,4 +596,18 @@ pub(crate) mod internal_bindings {
// `js_number(f64)` (i64 → f64 is lossless for the millisecond range).
Ok(JSValue::js_number(now as f64))
}

/// Monotonic milliseconds for deadlines Bun's own JS measures (e.g. the SQL
/// connect-retry budget). bun:test's `setSystemTime()` / `useFakeTimers()`
/// override `Date.now()` and `performance.now()` inside the engine, so a
/// deadline measured with either freezes or jumps along with the mocked
/// clock. This is the real clock the timer heap is drained against.
Comment thread
robobun marked this conversation as resolved.
Outdated
#[bun_jsc::host_fn]
pub(crate) fn monotonic_now_ms(
_global_this: &JSGlobalObject,
_call_frame: &CallFrame,
) -> JsResult<JSValue> {
let now = Timespec::now(TimespecMockMode::ForceRealTime).ms();
Ok(JSValue::js_number(now as f64))
}
}
88 changes: 87 additions & 1 deletion test/js/node/http/node-http-server-timeouts.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, test } from "bun:test";
import { describe, expect, setSystemTime, test } from "bun:test";
import { once } from "node:events";
import http from "node:http";
import net from "node:net";
Expand Down Expand Up @@ -218,3 +218,89 @@ describe("node:http server timeout enforcement", () => {
}
});
});

// The server notes when a connection's last response finished and, when the
// socket timer fires, grants whatever is left of the keep-alive budget from
// that mark. The mark has to be taken on the clock the timer runs on
// (monotonic), not on Date.now(): bun:test's setSystemTime() (and
// jest.useFakeTimers(), which overrides Date.now too) would otherwise decide
// when, or whether, an idle connection gets closed.
//
// Each probe moves Date.now() by an hour once the last response has arrived,
// then waits for the server to close the connection, timing it with
// performance.now(), which setSystemTime() leaves alone. Not concurrent:
// setSystemTime() is process-wide and the tests above time themselves with
// Date.now().
describe("keepAliveTimeout idle expiry ignores setSystemTime()", () => {
const KEEP_ALIVE_MS = 500;
// How long the server sits on the second request before answering it. The
// socket timer armed by the first response keeps counting down meanwhile,
// so it fires this much into the second response's idle period and the
// expiry has to be settled from the recorded mark.
const SLOW_RESPONSE_MS = 300;
const HOUR_MS = 60 * 60 * 1000;

async function probeIdleClose(options: { requests: 1 | 2; skewMs: number }) {
let requests = 0;
const server = http.createServer({ keepAliveTimeoutBuffer: 0 }, (req, res) => {
req.resume();
if (++requests === 2) {
setTimeout(() => res.end("response-body"), SLOW_RESPONSE_MS);
} else {
res.end("response-body");
}
});
server.keepAliveTimeout = KEEP_ALIVE_MS;
const port = await listen(server);
const socket = net.connect(port, "127.0.0.1");
try {
socket.setNoDelay(true);
socket.on("error", () => {});
const { promise: closed, resolve: onClosed } = Promise.withResolvers<void>();
socket.on("close", () => onClosed());
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

let received = "";
let responsesWanted = 0;
let onResponse = () => {};
socket.on("data", chunk => {
received += chunk.toString("latin1");
if (received.split("response-body").length - 1 >= responsesWanted) onResponse();
});
const request = () => {
const { promise, resolve } = Promise.withResolvers<void>();
responsesWanted++;
onResponse = resolve;
socket.write("GET / HTTP/1.1\r\nHost: a\r\n\r\n");
return promise;
};

await once(socket, "connect");
await request();
if (options.requests === 2) await request();
const lastResponseAt = performance.now();
setSystemTime(new Date(Date.now() + options.skewMs));
await closed;
return performance.now() - lastResponseAt;
} finally {
setSystemTime();
socket.destroy();
server.closeAllConnections();
server.close();
}
}

test("a clock moved forwards does not close the connection before its idle budget is used up", async () => {
// Settled against Date.now(), the timer fire 200ms into the second idle
// period sees an hour of idle time and closes the connection right there.
const idleMs = await probeIdleClose({ requests: 2, skewMs: HOUR_MS });
expect(idleMs).toBeGreaterThanOrEqual(KEEP_ALIVE_MS - 150);
});

test("a clock moved backwards does not keep the idle connection open", async () => {
// Settled against Date.now(), the timer fire re-arms the connection for
// the budget plus the hour the clock went back, and this probe only
// returns once the test times out.
const idleMs = await probeIdleClose({ requests: 1, skewMs: -HOUR_MS });
expect(idleMs).toBeGreaterThanOrEqual(KEEP_ALIVE_MS - 150);
});
});