Repository navigation
Conversation
`listen({ fd })` could not adopt a descriptor that a supervisor had
already bound and listened on, so systemd socket activation did not work:
systemd-socket-activate --listen=127.0.0.1:3000 node ./build # serves
systemd-socket-activate --listen=127.0.0.1:3000 bun ./build # hangs
`net.Server` surfaced a blanket "Bun does not support listening on a file
descriptor" EINVAL. `http.Server` was worse — it never read `fd` at all,
so it bound a fresh ephemeral port on `::` and emitted `listening`. The
process looked healthy while the socket the supervisor held was never
accepted on, with no error anywhere.
uSockets could create listeners but not adopt one, so add
`us_socket_group_listen_from_fd` next to the existing `listen`/`listen_unix`
and plumb it through: uws `HttpContext`/`App`, the C ABI, `Bun.listen`
(`UnixOrHost::Fd`), `Bun.serve` (`Address::Fd`), and `node:http`.
The fd is validated with SO_ACCEPTCONN before adoption. Polling a regular
file or a connected socket would never become acceptable, so without the
check an unusable descriptor would hang rather than report. ENOTSOCK and
EINVAL are now distinguished. Ownership transfers only once the listener
is live, unlike the paths that create their own fd.
`server.address()` reports the descriptor's real bound address, matching
Node, because port and hostname are read back from the live listener
rather than from config.
Fixes oven-sh#22559
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
WalkthroughChangesAdds socket activation support by adopting inherited listening file descriptors through the uSockets/uWebSockets layers, Bun’s server runtime, and Node-compatible Inherited listener support
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/server/mod.rs (1)
2863-2891: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate
Addr::Fdonhttp1before callinglisten_from_fd.Address::Fd is accepted with
{http3: true, http1: false}, but the inherited fd path silently destroys the HTTP/3 app and still attaches a listening TCP fd to the regular HTTP app. Match the TCP branch here so config validation is actually enforced.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/server/mod.rs` around lines 2863 - 2891, Update the address handling around the Addr enum and its downstream listen_from_fd path to reject or avoid Addr::Fd when http1 is false, matching the existing TCP branch’s HTTP/1 validation. Preserve inherited-fd behavior only for configurations with http1 enabled, preventing HTTP/3-only configurations from attaching the fd to the regular HTTP app.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/server/mod.rs`:
- Around line 1856-1869: Update the Address::Fd branch in on_listen_failed() to
receive and preserve the native errno from fd-adopt validation instead of
hardcoding EINVAL. Convert it using bun_sys::Error::from_code(errno,
bun_sys::Tag::listen).to_system_error(), while retaining the existing failure
message and fd context so non-socket and non-listening descriptors report their
distinct error codes.
In `@src/runtime/server/ServerConfig.rs`:
- Around line 1263-1277: Update the fd parsing block in the server configuration
function to reject fd when hostname or unix has already selected an address,
before assigning Address::Fd(fd). Return an invalid-arguments error for these
conflicting combinations, preserving the existing nonnegative fd validation and
allowing fd alone.
In `@test/js/node/http/node-http-listen-fd.test.ts`:
- Around line 19-23: Close the parent’s donor listener immediately after
Bun.spawn inherits file descriptor 3 and before issuing the request, while
preserving the child process setup in the test. Ensure the success tests await
observable completion and route failure conditions to rejection so they cannot
race or hang.
---
Outside diff comments:
In `@src/runtime/server/mod.rs`:
- Around line 2863-2891: Update the address handling around the Addr enum and
its downstream listen_from_fd path to reject or avoid Addr::Fd when http1 is
false, matching the existing TCP branch’s HTTP/1 validation. Preserve
inherited-fd behavior only for configurations with http1 enabled, preventing
HTTP/3-only configurations from attaching the fd to the regular HTTP app.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 892f5eae-4833-4a59-affc-a07bdacc042f
📒 Files selected for processing (13)
packages/bun-usockets/src/context.cpackages/bun-usockets/src/libusockets.hpackages/bun-uws/src/App.hpackages/bun-uws/src/HttpContext.hsrc/js/node/_http_server.tssrc/runtime/server/ServerConfig.rssrc/runtime/server/mod.rssrc/runtime/server/server_body.rssrc/runtime/socket/Listener.rssrc/uws_sys/App.rssrc/uws_sys/SocketGroup.rssrc/uws_sys/libuwsockets.cpptest/js/node/http/node-http-listen-fd.test.ts
Comment on lines
+1856
to
+1869
| server_config::Address::Fd(fd) => { | ||
| let err = jsc::SystemError { | ||
| errno: bun_sys::SystemErrno::EINVAL as i32, | ||
| code: bun_core::String::static_("EINVAL").into(), | ||
| message: bun_core::String::create_format(format_args!( | ||
| "Failed to listen on file descriptor {fd}: not a bound, listening socket" | ||
| )) | ||
| .into(), | ||
| syscall: bun_core::String::static_("listen").into(), | ||
| fd: *fd, | ||
| ..Default::default() | ||
| }; | ||
| err.to_error_instance(global) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files and inspect the cited server code plus nearby address/listen helpers.
printf 'Tracked files around server/mod.rs:\n'
git ls-files | rg '(^|/)src/runtime/server/mod\.rs$|(^|/)Listener\.rs$|(^|/)context\.c$|(^|/)context\.h$|(^|/).*listen.*fd.*\.(c|cpp|h|hpp|ts)$' || true
printf '\nserver/mod.rs line count:\n'
wc -l src/runtime/server/mod.rs 2>/dev/null || true
printf '\nRelevant server/mod.rs sections:\n'
sed -n '1830,1885p' src/runtime/server/mod.rs
printf '\n'
rg -n "enum Address|impl Address|Address::Fd|get_url_as_string|fn listen\(|listen_with_config|listen_from_fd|on_listen_failed|SystemErrno|get_errno" src/runtime/server/mod.rs
printf '\nListener.rs candidates:\n'
fd -a 'Listener\.rs$|listener\.rs$|net.*server|server.*net' . | sed 's#^\./##' | head -80Repository: oven-sh/bun
Length of output: 6759
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'src/runtime/server/mod.rs: listen() Address dispatch\n'
sed -n '2870,3045p' src/runtime/server/mod.rs
printf '\nsrc/runtime/server/mod.rs: get_url_as_string fd handling\n'
sed -n '495,545p' src/runtime/server/mod.rs
printf '\n'
sed -n '1900,1945p' src/runtime/server/mod.rs
printf '\npackages/bun-usockets/src/context.c listen_from_fd snippets\n'
wc -l packages/bun-usockets/src/context.c
rg -n "listen_from_fd|us_socket_group_listen_from_fd|ERROR|ENOTSOCK|EINVAL|SO_ACCEPTCONN|errno" packages/bun-usockets/src/context.c
sed -n '1,120p' packages/bun-usockets/src/context.c
sed -n '120,230p' packages/bun-usockets/src/context.c
printf '\nsrc/runtime/socket/Listener.rs FD/errno handling\n'
rg -n "listen_from_fd|Addr::Fd|errno|Fd\\(|E::ENOTSOCK|E::EINVAL|get_errno|to_error" src/runtime/socket/Listener.rs
sed -n '1,260p' src/runtime/socket/Listener.rs
printf '\nRelevant test expectations\n'
sed -n '1,220p' test/js/node/http/node-http-listen-fd.test.tsRepository: oven-sh/bun
Length of output: 41746
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'context.c listen_from_fd full implementation:\n'
sed -n '427,475p' packages/bun-usockets/src/context.c
printf '\ncontext.c App::listen_from_fd calls around 350-420:\n'
sed -n '340,418p' packages/bun-usockets/src/context.c
printf '\nRead-only verifier extracting errno assignments and call path:\n'
python3 - <<'PY'
from pathlib import Path
import re
p = Path("packages/bun-usockets/src/context.c")
src = p.read_text()
m = re.search(r"struct us_listen_socket_t \*us_socket_group_listen_from_fd\([^}]+(?:\n[^{}]+)*\n\}", src, re.S)
print("listeners:", bool(m))
if m:
print(m.group(0))
for label in ["SO_ACCEPTCONN", "BUN_USOCKETS_BLOCKING", "bsd_set_nonblocking", "save_set_sockopts", "listen_with_options"]:
print(f"{label}: ", bool(re.search(label, src[m.start():m.end()], re.S)))
for expr in re.findall(r"(int|static|int .*int)\s+([^;\n]+?\([^)]*\)),\s*saved_errno|= saved_errno|(\*error\s*=\s*saved_errno|errno\s*=\s*errno|error\s*=\s*errno|errno\s*=.*?)", m.group(0), re.S):
print(expr)
PYRepository: oven-sh/bun
Length of output: 5227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'bun-uws bindings for listen_from_fd signatures:\n'
rg -n "listen_from_fd|listen_from_fd_rc|LIBUS_SOCKET_ERROR|typedef.*LIBUS_SOCKET_DESCRIPTOR|fn listen_from_fd|listen_from_fd\\(" packages src -g '*.{rs,h,hpp,hpp,hxx,clang}' 2>/dev/null || true
rg -n "listen_from_fd" . --type rust --type h --type cpp --type c 2>/dev/null | head -200
printf '\nSearch server on_listen trampoline for errno args:\n'
rg -n "on_listen|on_listen_failed|us_socket.*listen|listen_from_fd.*error|us_socket.*listen_from_fd" src/runtime/server/mod.rsRepository: oven-sh/bun
Length of output: 4635
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'uws_sys/libuwsockets.cpp listen_from_fd wrapper:\n'
sed -n '516,538p' src/uws_sys/libuwsockets.cpp
printf '\nsrc/runtime/server/mod.rs on_listen trampoline:\n'
sed -n '3185,3208p' src/runtime/server/mod.rsRepository: oven-sh/bun
Length of output: 2177
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'pkg exports / generated wrapper availability:\n'
git ls-files | rg '(^|/)libuwsockets\.(h|cpp|c|h|hpp)$|(^|/)libuwsockets[0-9]*\.(h|cpp|c|h|hpp)$|(^|/)generated.*uw|(^|/)packages/bun-usockets/src/libuwsockets'
printf '\nRead-only search for uws_app_listen_from_fd declarations and error handling:\n'
rg -n "uws_app_listen_from_fd|listen_from_fd_rc|listen_from_fd.*error|handler\\(\([^\\n]+nullptr|app->listen_from_fd" packages src/uws_sys src/uws -g '*.{h,hpp,hxx,c,cpp,rs}' 2>/dev/null || trueRepository: oven-sh/bun
Length of output: 798
Preserve the real errno for Address::Fd listen failures.
Native fd-adopt validation already distinguishes ENOTSOCK from EINVAL, but Bun.serve()’s Address::Fd arm always reports EINVAL in on_listen_failed(). Wire the real errno through the fd listen failure path and report bun_sys::Error::from_code(errno, bun_sys::Tag::listen).to_system_error() so non-listening/non-socket descriptors get the expected ECODE distinction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/server/mod.rs` around lines 1856 - 1869, Update the Address::Fd
branch in on_listen_failed() to receive and preserve the native errno from
fd-adopt validation instead of hardcoding EINVAL. Convert it using
bun_sys::Error::from_code(errno, bun_sys::Tag::listen).to_system_error(), while
retaining the existing failure message and fd context so non-socket and
non-listening descriptors report their distinct error codes.
Comment on lines
+1263
to
+1277
| if let Some(fd_js) = arg.get_truthy(global, "fd")? { | ||
| if fd_js.is_number() { | ||
| let fd = fd_js.to_int32(); | ||
| if fd < 0 { | ||
| return Err( | ||
| global.throw_invalid_arguments(format_args!("Expected \"fd\" to be >= 0")) | ||
| ); | ||
| } | ||
| args.address = Address::Fd(fd); | ||
| } | ||
| } | ||
| if global.has_exception() { | ||
| return Err(JsError::Thrown); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Crash: hostname + fd (or unix + fd) reaches an unreachable!() panic.
This block unconditionally overwrites args.address with Address::Fd(fd) without checking whether has_hostname was already set or whether args.address is already Address::Unix. Later in this same function (unchanged code, ~line 1572), the no-baseURI branch does:
let hostname: &[u8] = if has_hostname {
match &args.address {
Address::Tcp { hostname, .. } => hostname.as_ref().unwrap().as_bytes(),
_ => unreachable!(),
}
} else { b"0.0.0.0" };Calling Bun.serve({ hostname: "example.com", fd: 3, fetch }) (no baseURI) sets has_hostname = true, then fd parsing overwrites args.address to Address::Fd, so this hits unreachable!() and panics the whole process. The same silent-precedence problem applies to unix + fd (no error, unlike the explicit hostname+unix check a few lines above).
💥 Proposed fix: reject conflicting address options when `fd` is set
if let Some(fd_js) = arg.get_truthy(global, "fd")? {
if fd_js.is_number() {
let fd = fd_js.to_int32();
if fd < 0 {
return Err(
global.throw_invalid_arguments(format_args!("Expected \"fd\" to be >= 0"))
);
}
+ if has_hostname {
+ return Err(global.throw_invalid_arguments(format_args!(
+ "Cannot specify both hostname and fd",
+ )));
+ }
+ if matches!(args.address, Address::Unix(_)) {
+ return Err(global.throw_invalid_arguments(format_args!(
+ "Cannot specify both unix and fd",
+ )));
+ }
args.address = Address::Fd(fd);
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/server/ServerConfig.rs` around lines 1263 - 1277, Update the fd
parsing block in the server configuration function to reject fd when hostname or
unix has already selected an address, before assigning Address::Fd(fd). Return
an invalid-arguments error for these conflicting combinations, preserving the
existing nonnegative fd validation and allowing fd alone.
Comment on lines
+19
to
+23
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), path.join(String(dir), "fixture.js")], | ||
| env: bunEnv, | ||
| stdio: ["ignore", "pipe", "pipe", fd], | ||
| }); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Close the donor listener after spawning the child.
The parent still polls the shared listener, so it can accept the fetch itself and leave it unanswered. This makes both success tests race and potentially hang. Close the parent copy after Bun.spawn has inherited fd 3, before issuing the request.
Proposed fix
await using proc = Bun.spawn({
cmd: [bunExe(), path.join(String(dir), "fixture.js")],
env: bunEnv,
stdio: ["ignore", "pipe", "pipe", fd],
});
+ await new Promise<void>((resolve, reject) =>
+ donor.close(error => (error ? reject(error) : resolve())),
+ );
try {
@@
} finally {
proc.kill();
- donor.close();
}As per coding guidelines, tests must await observable conditions and wire failure paths to rejection.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await using proc = Bun.spawn({ | |
| cmd: [bunExe(), path.join(String(dir), "fixture.js")], | |
| env: bunEnv, | |
| stdio: ["ignore", "pipe", "pipe", fd], | |
| }); | |
| await using proc = Bun.spawn({ | |
| cmd: [bunExe(), path.join(String(dir), "fixture.js")], | |
| env: bunEnv, | |
| stdio: ["ignore", "pipe", "pipe", fd], | |
| }); | |
| await new Promise<void>((resolve, reject) => | |
| donor.close(error => (error ? reject(error) : resolve())), | |
| ); | |
| try { | |
| @@ | |
| } finally { | |
| proc.kill(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/js/node/http/node-http-listen-fd.test.ts` around lines 19 - 23, Close
the parent’s donor listener immediately after Bun.spawn inherits file descriptor
3 and before issuing the request, while preserving the child process setup in
the test. Ensure the success tests await observable completion and route failure
conditions to rejection so they cannot race or hang.
Source: Coding guidelines
Collaborator
|
Thank you for this work. #44013 implements the same feature on top of Two behaviours differ:
I did not close this PR. A maintainer decides which one lands. If #44013 misses a case that this PR handles, please say so there. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes #22559 — makes systemd socket activation actually work, rather than fail more loudly.
Two different failures were behind this:
net.Server.listen({ fd })returned a blanketEINVAL — Bun does not support listening on a file descriptor.http.Server.listen({ fd })never readfdat all. It bound a fresh ephemeral port on::and emittedlistening, so the process reported healthy and passed liveness checks while the socket the supervisor was holding never got accepted on — no error anywhere. This is the one the reporter hit, via the SvelteKit node adapter.Relationship to #33578
@robobun's #33578 fixes the second half by making
httpreportEINVALlikenetdoes, noting "uSockets exposes no listen-from-fd entry point". That's accurate for the tree as it stands — so this PR adds that entry point and adopts the descriptor instead. It supersedes #33578 and touches the same test path; happy to rebase onto it or close this if you'd rather land the error-surfacing fix first.Cause
uSockets could create listeners (
us_socket_group_listen,us_socket_group_listen_unix) but not adopt one. There was precedent for connected sockets (us_socket_from_fd) and UDP (us_create_udp_socket_from_fd), just nothing for a TCP listener.Fix
Adds
us_socket_group_listen_from_fdalongside the existing two and plumbs it through the layers: uwsHttpContext/App→ C ABI (uws_app_listen_from_fd) →Bun.listen(UnixOrHost::Fd) →Bun.serve(Address::Fd) →node:http.Three things worth reviewer attention:
SO_ACCEPTCONNbefore adoption. Polling a regular file or a connected socket would never become acceptable, so without this an unusable descriptor hangs instead of reporting — the exact failure mode being fixed.ENOTSOCK(not a socket) andEINVAL(a socket, but not listening) are now distinguished.server.address()returns the descriptor's real bound address ({"address":"127.0.0.1","family":"IPv4","port":18760}), matching Node, because port/hostname are read back from the live listener rather than from config.Address::Fdcontributes neither.http3: truewith an inherited fd skips the H3 listener with a warning — the inherited descriptor is a stream socket and QUIC needs its own UDP socket, which the supervisor did not hand us. Same shape as the existing unix-socket behaviour.How did you verify your code works?
Real systemd, end to end, with the debug build:
Before this change the same command reported
address={"address":"::","port":40069}and curl hung.Three tests in
test/js/node/http/node-http-listen-fd.test.ts, coveringhttp,net, and the rejection path. They don't require systemd: a donor server binds127.0.0.1:0, and the child inherits its own copy of the descriptor as fd 3 viastdio— the same handoff systemd performs, so the child gets a distinct fd and there's no shared-ownership close. The child'slisteningcallback is awaited rather than slept on.Verified the tests are real by reverting the source changes and rebuilding — all 3 fail:
The rejection test fails differently pre-fix (
EINVALfrom the old blanket error vsENOTSOCKnow), which is the added diagnostic precision.With the fix:
3 pass. Regression run overnode-http.test.ts+serve.test.ts: 413 pass, 2 skip, 1 fail. The one failure isserver.requestIP > v6, which I confirmed fails identically on a clean tree (it binds::1but fetcheslocalhost, which resolves to127.0.0.1first on this box) — pre-existing and unrelated.cargo clippy -p bun_runtime -p bun_uws_sys --no-depsis clean.Not covered: executed on Linux x64 only. The C path is POSIX-shaped and
SO_ACCEPTCONNexists on macOS and Winsock, but I have not run it on either, and the tests areskipIf(!isPosix).Update on the Windows typing question I raised earlier: I flagged uncertainty about the
getsockoptoptlen type on Windows. Resolved by inspection —packages/bun-usockets/src/internal/networking/bsd.hincludesws2tcpip.hon Windows, which typedefssocklen_tasint, matching Winsock'sgetsockopt(SOCKET, int, int, char *, int *). The(char *)cast andsocklen_tout-param are correct on both platforms, andstruct bsd_addr_tin that same header already usessocklen_tunguarded. So this should compile and behave on Windows; it still has not been run there.