Bun.serve, node:http: run request handlers with the event loop entered

Merged
RO
3mo ago
+405-2

Problem

  • In a fetch(), route, or node:http request handler, server.upgrade() (which runs open() before it returns) or ws.close() (which runs close()) drains the microtask queue in the middle of the handler: then(f); ws.close(); g() runs f before g. Node runs f after the handler returns. Async continuations and abort listeners are affected too.
  • Cause: the dispatch paths under src/runtime/server/ (list in Notes) call into JS with entered_event_loop_count at 0 and drain explicitly afterwards. The enter()/exit() pair around open() or close() (ServerWebSocket.rs) is then the outermost one, so EventLoop::exit() drains. The explicit drains ran at 0 too.

Fix

  • Adds EventLoop::enter_scope_without_checkpoint(): an enter() whose exit only balances the count. Each dispatch path holds it across the handler call and its explicit drains.
  • Every other dispatcher runs callbacks with the count above 0 (tick(), timers, sockets). Under the scope the nested pair is not the outermost one and does not drain. The queued callbacks run at the drain the dispatcher performs anyway, so the checkpoints per request are unchanged. Matches Node.
  • Verified: 9 new cases in websocket-server.test.ts, node-http-with-ws.test.ts and serve-http3.test.ts, all failing on current bun. Also ran the websocket, http, node/http, ws and bake suites.

Background

  • entered_event_loop_count (src/jsc/event_loop.rs) is the depth of native entries into JS. exit() runs the microtask checkpoint (nextTick callbacks, then promise jobs) only for the outermost exit. This gives each callback run-to-completion.
  • The request paths drain explicitly: RequestContext::on_response drains before it looks at the returned value, so a promise the drain settled is unwrapped without a tick. Hence a scope that exits without a checkpoint.
Notes

Dispatch paths changed: mod.rs on_request, on_user_route_request, on_node_http_request_with_upgrade_ctx, on_saved_request (bake); server_body.rs on_web_socket_upgrade, upgrade_web_socket_user_route, on_request_for and on_user_route_request_for (HTTP/3); RequestContext::on_abort.

Cost: on a release build, the difference in the disassembly of NewServer::on_request is two loads, an inc of entered_event_loop_count before the handler call and a dec after it, plus a push/pop of the register that holds the pointer. The node:http dispatch gets the same and still contains its two calls to drain_microtasks_with_global. No drain call is added or removed on any path. Details in the comments below.

Tests: 6 cases in test/js/bun/websocket/websocket-server.test.ts (fetch and route, each calling upgrade() and closing a socket; an abort listener; an async continuation), 2 in test/js/node/http/node-http-with-ws.test.ts (a ws connection handler run from the upgrade request, and a request handler closing a socket), 1 in test/js/bun/http/serve-http3.test.ts (fetch and a route over HTTP/3).

Repro on bun 1.4.0 (prints ["microtask","after"], expected ["after"]):

const server = Bun.serve({
  port: 0,
  fetch(req, server) {
    const order = [];
    Promise.resolve().then(() => order.push("microtask"));
    server.upgrade(req);
    order.push("after");
    console.log(JSON.stringify(order));
  },
  websocket: { open() {}, message() {} },
});
new WebSocket(`ws://127.0.0.1:${server.port}`).onopen = () => server.stop(true);

The same code in a setTimeout callback prints the expected order, because timers run under enter()/exit(). The async variant (await null first, then the same statements) also printed the wrong order: the continuation ran from the explicit drain in on_response, with the count at 0. The two node:http cases print ["rest of handler","nextTick","microtask"] on Node 26 and printed ["nextTick","microtask","rest of handler"] on bun.

Out of scope, unchanged:

  • The explicit drains are unconditional. server.stop(true) called from inside a handler still runs on_abort, and its drain, inside the handler. Making every explicit drain observe the count would be a change to drain_microtasks_with_global, the hottest function of the request path, and would affect every subsystem that drains explicitly.
  • Evaluation of the main module also runs with the count at 0.
  • The bake on_saved_request path gets the same one-line change but has no test here, since it needs a framework app. The bake framework suites (react-response, request-cookies, response-to-bake-response) pass.

Suites run with the debug build: test/js/bun/websocket/, test/js/bun/http/serve.test.ts, bun-serve-routes.test.ts, bun-server.test.ts, serve-http3.test.ts, the abort and stream related serve-*.test.ts files, test/js/node/http/, test/js/first_party/ws/, the ws regression tests, and the three bake files above. The failures seen were the same with and without this change: the send() (benchmark) test in websocket-server.test.ts starves its concurrent neighbours on a debug build in this container, and a few tests depend on localhost resolution, on not running as root, or on an empty proxy environment. They fail on stock bun here too.