-
Notifications
You must be signed in to change notification settings - Fork 5k
Rewrite the uSockets core in Rust #34037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
969426c
8afe56d
f2153e5
31de3b3
362271c
bc538a0
bd53641
440b7f7
ba0797d
c7beeb0
6458437
f2c43af
1e9af71
aae3238
0ebb637
bef5ff3
7fa426e
76786b4
a635dc6
39e9610
d0b8e1a
34bc78b
6a44c29
2db9e78
a91ffd3
8dc8a56
92d331d
0a297a2
8dd10b9
027d513
3bfaf56
873c33a
f3dcf31
9920e0e
29b3121
73af3d0
b16e5fd
93d9e15
78b5142
ab20e85
0c66675
42d0066
c72b792
ee37e3a
b061f12
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,18 +76,19 @@ | |
| "src/mimalloc_sys", | ||
| "src/semver_jsc", | ||
| "src/standalone_graph", | ||
| "src/uws", | ||
| "src/wyhash", | ||
| "src/hash", | ||
| "src/boringssl_sys", | ||
| "src/bssl", | ||
| "src/bun_core", | ||
| "src/bun_core_macros", | ||
| "src/bun_output_tags", | ||
| "src/patch_jsc", | ||
| "src/router", | ||
| "src/shell_parser", | ||
| "src/tcc_sys", | ||
| "src/uws_sys", | ||
| "src/usockets", | ||
|
Check warning on line 90 in Cargo.toml
|
||
|
claude[bot] marked this conversation as resolved.
claude[bot] marked this conversation as resolved.
claude[bot] marked this conversation as resolved.
claude[bot] marked this conversation as resolved.
claude[bot] marked this conversation as resolved.
claude[bot] marked this conversation as resolved.
claude[bot] marked this conversation as resolved.
claude[bot] marked this conversation as resolved.
claude[bot] marked this conversation as resolved.
claude[bot] marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 🟡 Three more stale references to files this PR deletes, in files the earlier grep-and-update passes missed because two of them are outside the diff: Extended reasoning...What the issue isThree doc comments describe a live relationship with files this PR deletes:
Why the earlier passes missed themThis is the same finding class as the previously-accepted-and-fixed nits on this PR: #4 (lib.rs:12-14 + cabi.rs:12-16 → SocketKind.rs / "live C"), #8 (README.md → epoll_kqueue.c), and #13 (lib.rs:40 → "feature-gated OFF"). Those fix passes covered files the PR touches. The first two files here — ImpactNo runtime effect — pure doc staleness, hence nit. The Step-by-step proof
How to fix
(Filed against Cargo.toml because
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The doc comments at Extended reasoning...What the issue isTwo doc comments describe
But Why the implementation is correct (C-parity)The deleted C function this ports is /* Try and match by wildcard */
it = root->children.find("*");
if (it == root->children.end()) {
return nullptr;
}
return getUser(it->second.get(), label + 1, labels, numLabels);So the Rust impl exactly matches the deleted C — the doc comments are what's wrong. Why the CodeRabbit suggestion should NOT be appliedThere is an unresolved CodeRabbit inline comment at Applying that would be a regression. The sole caller of static struct ssl_ctx_st *onMissingServerName(..., const char *hostname, ...) {
...
/* The handler is expected to have registered the name via addServerName();
* hand the newly-registered context back so the in-flight handshake uses it */
return us_listen_socket_find_server_name_ctx(ls, hostname);
}Here ImpactNo runtime effect — the implementation is correct today; only the two doc comments are wrong. Hence nit. But the finding's real value is flagging that the still-open CodeRabbit suggestion is backwards: applying it (which the "🤖 Prompt for AI Agents" block invites) would silently regress a user-visible flow that the deleted C supported and that has no test coverage in this PR. Step-by-step proof
How to fix
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 🟡 Companion to still-open #19 ( Extended reasoning...What the issue is
All three are references into the author's local Relationship to still-open comment #19Comment #19 (still unresolved, filed at Verified: Why nothing prevents itThese are ImpactNo runtime effect — pure comment staleness, hence nit. The concrete downside: a reader trying to understand why Step-by-step proof
How to fixDrop the three parentheticals — the sentences read fine without them:
The (Filed against workspace |
||
| "src/uws_shim", | ||
| "src/zstd", | ||
| "src/options_types", | ||
| "src/output", | ||
|
|
@@ -181,11 +182,10 @@ | |
| # back to "warn" (their priority 0 beats the group's -1) where a warning level | ||
| # is intentional. | ||
| warnings = { level = "deny", priority = -1 } | ||
| # `bun_asan` / `bun_debug` / `socket_fault_injection` are set via RUSTFLAGS | ||
| # (`--cfg=...` + `--check-cfg=cfg(...)`) by scripts/build/rust.ts; register | ||
| # them here so a plain `cargo build` / `cargo check` (without those flags) | ||
| # doesn't warn. | ||
| unexpected_cfgs = { level = "warn", check-cfg = ['cfg(bun_asan)', 'cfg(bun_debug)', 'cfg(socket_fault_injection)'] } | ||
| # `bun_asan` / `bun_debug` are set via RUSTFLAGS (`--cfg=...` + | ||
| # `--check-cfg=cfg(...)`) by scripts/build/rust.ts; register them here so a | ||
| # plain `cargo build` / `cargo check` (without those flags) doesn't warn. | ||
| unexpected_cfgs = { level = "warn", check-cfg = ['cfg(bun_asan)', 'cfg(bun_debug)'] } | ||
| # link.exe unconditionally prints "Creating library X.dll.lib and object | ||
| # X.dll.exp" to stdout when linking each proc-macro DLL on Windows hosts; | ||
| # there is no linker flag to suppress it. The lint already exempts itself | ||
|
|
@@ -423,10 +423,10 @@ | |
| bun_mimalloc_sys = { path = "src/mimalloc_sys" } | ||
| bun_semver_jsc = { path = "src/semver_jsc" } | ||
| bun_standalone_graph = { path = "src/standalone_graph" } | ||
| bun_uws = { path = "src/uws" } | ||
| bun_wyhash = { path = "src/wyhash" } | ||
| bun_hash = { path = "src/hash" } | ||
| bun_boringssl_sys = { path = "src/boringssl_sys" } | ||
| bun_bssl = { path = "src/bssl" } | ||
| bun_core = { path = "src/bun_core" } | ||
| bun_core_macros = { path = "src/bun_core_macros" } | ||
| bun_output_tags = { path = "src/bun_output_tags" } | ||
|
|
@@ -436,7 +436,8 @@ | |
| bun_spawn = { path = "src/spawn" } | ||
| bun_spawn_sys = { path = "src/spawn_sys" } | ||
| bun_tcc_sys = { path = "src/tcc_sys" } | ||
| bun_uws_sys = { path = "src/uws_sys" } | ||
| bun_usockets = { path = "src/usockets" } | ||
| bun_uws_shim = { path = "src/uws_shim" } | ||
| bun_zstd = { path = "src/zstd" } | ||
| bun_options_types = { path = "src/options_types" } | ||
| bun_bunfig = { path = "src/bunfig" } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.