Skip to content

Remove dead code from node:crypto bindings, JSC/WebCore bindings, uSockets, and llhttp - #37454

Open
robobun wants to merge 6 commits into
mainfrom
claude/farm/1fb20db1/dead-code-linker-sweep
Open

Remove dead code from node:crypto bindings, JSC/WebCore bindings, uSockets, and llhttp#37454
robobun wants to merge 6 commits into
mainfrom
claude/farm/1fb20db1/dead-code-linker-sweep

Conversation

@robobun

@robobun robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Removes 1,302 lines of code that nothing references (16 lines added, all import, signature and comment adjustments), across the C++ JSC bindings, node:crypto helpers, the in-tree uSockets fork, llhttp, and a few Rust FFI wrappers. No behavior change: every removed function was discarded by the linker, i.e. the shipped binary never contained it.

How the candidates were found

Previous sweeps were grep-driven, so this run used the linker instead: the debug binary was relinked with -Wl,--gc-sections -Wl,--print-gc-sections and the discarded .text.* sections from bun's own objects (minus anything still present in the output) gave ~4,300 symbols that nothing references at link time. Those were filtered down by

On the Rust side every crate was additionally checked by demoting its pub items to pub(crate) and letting rustc's dead_code analysis run; almost everything it reported was already claimed by open PRs, so only the items below are included.

Removed

node:crypto (CryptoUtil.*, ncrypto.*)

  • keyFromString, passphraseFromBufferSource, and the header overloads of parseKeyFormat / parseKeyType (the ThrowScope overloads are the live ones); ByteSource::fromBIO, ByteSource::foreign; the BunString.h include they needed
  • Rsa::encrypt / Rsa::decrypt and the RSA_Cipher template behind them (the live path is Cipher::encrypt/decrypt), plus EVPKeyCtxPointer::setRsaMgf1Md, which only they called
  • Cipher::ForEach with its CipherCallbackContext / array_push_back helpers and CipherNameCallback alias (node_crypto_binding.cpp calls EVP_CIPHER_do_all_sorted directly)
  • checkScryptParams, scrypt, pbkdf2 (scrypt/pbkdf2 go through the Rust bindings), X509View::ifRsa / ifEc and their KeyCallback alias, BignumPointer::isOne, BIOPointer::NewFp

JSC / WebCore bindings

  • InspectorHTTPServerAgent::requestWillBeSent / responseReceived / bodyChunkReceived / requestFinished / requestHandlerException (their Rust callers were removed in an earlier sweep)
  • ScriptExecutionContext::ensureOnMainThread, free function executionContext(JSGlobalObject*)
  • JSCStackFrame::typeName / retrieveTypeName / m_typeName (the neighbouring getStackTraceForThrownValue is removed by Error.prepareStackTrace: index source URLs by visible frame, not by JSC frame #37450)
  • PerformanceResourceTiming::create and its constructor, the entryStartTime / entryEndTime helpers, and ResourceTiming::populateServerTiming, which only the constructor used (the class is still exposed as a global constructor; nothing ever instantiates it natively)
  • rejectPromiseWithGetterTypeError and the CastedThisErrorBehavior::RejectPromise branch of IDLAttribute::get that was its only (never instantiated) user
  • JSErrorHandler.cpp / JSErrorHandler.h (whole files) and the setAttributeEventListener<JSErrorHandler> instantiation: no attribute listener is ever created with it
  • callbackData() and the native-to-JS toJS(...) overloads of JSAbortAlgorithm and JSPerformanceObserverCallback (only the JS-to-native direction is used)
  • JSBuffer.cpp: the jsBufferConstructorAlloc*WithoutTypeChecks DOMJIT operations (their signatures went away in Fix incorrect include order #9457) and the JSValue-named validateOffset overload

uSockets (packages/bun-usockets) and its Rust wrappers

  • us_socket_is_tls, us_socket_detach, us_connecting_socket_get_loop, us_socket_pair (+ SocketGroup::pair), us_socket_open (+ us_socket_t::open), us_listen_socket_ext, us_listen_socket_port, us_listen_socket_get_fd (+ ListenSocket::fd), us_socket_group_next, us_socket_group_timestamp, us_loop_iteration_number (Rust reads iteration_nr from the struct directly), us_poll_ext
  • us_listen_socket_find_server_name_userdata and the now write-only sni_node_t::user field (userdata is looked up through the SSL_CTX ex_data by us_socket_server_name_userdata)
  • us_udp_packet_buffer_local_ip / bsd_udp_packet_buffer_local_ip
  • QUIC: us_quic_pending_connect_user together with the now write-only user field it read and the void *user parameter of us_quic_socket_context_connect / Context::connect that only fed it, the us_quic_socket_context accessor, us_quic_stream_flush, us_quic_stream_has_unacked, us_quic_socket_close (+ quic::Socket::close), the never-set on_open callback (field, setter and dispatch), us_nq_spec_peer_ctx (+ its Rust declaration)

llhttp (src/jsc/bindings/node/http/llhttp, already trimmed in #36576)

  • the __wasm__-only block (llhttp_alloc / llhttp_free and the wasm settings table; bun never builds this file for wasm), the llhttp_get_* accessors, llhttp_reset, llhttp_settings_init, llhttp_method_name, llhttp_status_name, llhttp__debug

Rust

  • env_var.rs: BUN_DUMP_STATE_ON_CRASH (bake's crash-state dump; BUN_NEEDS_PROC_SELF_WORKAROUND, MI_VERBOSE and TODIUM are in the same state but already removed by Remove dead code from C++ bindings, src/js builtins, CSS, and Rust util crates #35437). Its reader lived in the Zig sources and was not carried over in the Rust rewrite, so nothing has read it since May; if the feature is wanted back it needs to be reimplemented, not just declared.
  • node_fs.rs: re-exports of CpSingleTask, IntoResultListEntry, ResultListEntry, ResultListEntryValue that nothing imported (the items themselves are still used inside the module and become pub(crate)).

test/internal/source-lints/dead-symbols-linker-sweep.test.ts pins every removed symbol so it cannot quietly come back; it fails on main and passes here.

Verification

  • bun bd (full debug build) passes; bun run rust:check-all passes on all 10 target triples; cargo fmt --check and clang-format are clean.
  • bun bd test passes on test/js/node/crypto/{node-crypto,crypto-rsa,crypto.key-objects,scrypt,pbkdf2,x509}, test/js/node/http/node-http-parser.test.ts, test/js/bun/udp/udp_socket.test.ts, test/js/node/quic/quic-stream.test.ts, test/js/web/abort/abort.test.ts, test/js/node/perf_hooks/perf_hooks.test.ts, test/js/node/v8/capture-stack-trace.test.js, test/regression/issue/prepare-stack-trace-crash.test.ts, test/js/node/buffer.test.js, and bun bd test/js/node/test/parallel/test-tls-sni-server-client.js exits 0. test/js/bun/http/serve.test.ts has the same 4 failures as the released binary in this container (IPv6, privileged ports, non-loopback clients); everything else in it passes.
  • No open PR adds a caller of anything removed here (checked the diffs of the open crypto/tls and http/socket/quic PRs). usockets: rewrite in Rust #33933, the uSockets-in-Rust rewrite, ports the same unused uSockets accessors 1:1; whichever lands second just drops them there too. usockets(kqueue): deliver a peer reset to sockets polling for no events #37101 cites us_socket_detach in a comment as an example of a detach path; that comment can be reworded when it lands.

Left alone on purpose (follow-ups, not in this diff)

  • DOMPromise::whenPromiseIsSettled -> DeferredPromise::whenSettled chain (~85 lines) is dead but its last link is removed by Remove dead code from webcore bindings, watcher, node-fallbacks, and misc crates #37062; it can go once that lands.
  • A handful of items that became unused in Worker / worker_threads: WebCore-shaped lifetimes, joined threads, one ordered VM teardown #37075 three days ago (JSPromise::resolve_task / settle_task, VM::has_termination_request + JSC__VM__hasTerminationRequest, Completion::off_thread, JsThread-witness accessors in job.rs) are left for that work to settle.
  • ServerTiming.h/.cpp, ResourceTiming's constructor and PerformanceServerTiming::create are now only reachable from each other; removing them means deciding what to do with the still-exposed PerformanceServerTiming constructor.
  • The <Class>_getter custom getters that the FOR_EACH_... macro in ZigGlobalObject.cpp emits for ~40 classes that never install them are discarded by the linker; dropping them needs the macro split, not a deletion.
  • PerformanceResourceTiming itself, sqlite3.c's RBU/session extensions, and the Loose/Clamp JSDOMConvertNumbers instantiations are unreferenced but are either exposed API surface, vendored, or supported generator options.

[review] gate passed · iteration 1 · 50 files touched

fails on main (without fix)
ASAN without fix: 5 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/internal/source-lints/dead-symbols-linker-sweep.test.ts
bun test v1.4.0 (c75475c68)

test/internal/source-lints/dead-symbols-linker-sweep.test.ts:
70 |       ["src/jsc/bindings/ncrypto.h", /\bifRsa\b/],
71 |       ["src/jsc/bindings/ncrypto.h", /\bifEc\b/],
72 |       ["src/jsc/bindings/ncrypto.h", /\bisOne\b/],
73 |       ["src/jsc/bindings/ncrypto.h", /\bNewFp\b/],
74 |     ]),
75 |   ).toEqual([]);
         ^
error: expect(received).toEqual(expected)

- []
+ [
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: \bkeyFromString\b",
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: \bparseKeyFormat\b",
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: \bparseKeyType\b",
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: \bpassphraseFromBufferSource\b",
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: \bfromBIO\b",
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: static ByteSource foreign\b",
+   "src/jsc/bindings/ncrypto.cpp: \bRSA_Cipher\b",
+   "src/jsc/bindings/ncrypto.cpp: \bRsa::(encrypt|decrypt)\b",
+   "src/jsc/bindings/ncrypto.h: \bse
... (truncated)

release without fix: 5 FAILED
bun test v1.4.0-canary.1 (287720910)

test/internal/source-lints/dead-symbols-linker-sweep.test.ts:
70 |       ["src/jsc/bindings/ncrypto.h", /\bifRsa\b/],
71 |       ["src/jsc/bindings/ncrypto.h", /\bifEc\b/],
72 |       ["src/jsc/bindings/ncrypto.h", /\bisOne\b/],
73 |       ["src/jsc/bindings/ncrypto.h", /\bNewFp\b/],
74 |     ]),
75 |   ).toEqual([]);
         ^
error: expect(received).toEqual(expected)

- []
+ [
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: \bkeyFromString\b",
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: \bparseKeyFormat\b",
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: \bparseKeyType\b",
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: \bpassphraseFromBufferSource\b",
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: \bfromBIO\b",
+   "src/jsc/bindings/node/crypto/CryptoUtil.h: static ByteSource foreign\b",
+   "src/jsc/bindings/ncrypto.cpp: \bRSA_Cipher\b",
+   "src/jsc/bindings/ncrypto.cpp: \bRsa::(encrypt|decrypt)\b",
+   "src/jsc/bindings/ncrypto.h: \bsetRsaMgf1Md\b",
+   "src/jsc/bindings/ncrypto.h: \bCipherNameCallback\b",
+   "src/jsc/bindings/ncrypto.cpp: \bCipher::ForEach\b",
+   "src/jsc/bindings/ncrypto.h: \bcheckScryptParams\b"
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/internal/source-lints/dead-symbols-linker-sweep.test.ts
bun test v1.4.0 (c75475c68)

test/internal/source-lints/dead-symbols-linker-sweep.test.ts:
(pass) dead node:crypto helpers (CryptoUtil, ncrypto) do not reappear [32.41ms]
(pass) dead JSC/WebCore binding helpers do not reappear [26.49ms]
(pass) orphaned files stay deleted [518.39ms]
(pass) dead uSockets entry points and their Rust wrappers do not reappear [34.34ms]
(pass) unused llhttp API surface does not reappear [13.47ms]
(pass) environment variables nothing reads any more are not re-declared [2.87ms]

 6 pass
 0 fail
 6 expect() calls
Ran 6 tests across 1 file. [3.06s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 740ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/121] gen JSBuffer.lut.h
Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[2/121] gen generated_host_exports.rs
generated_host_exports.rs: 93 exports (host=3, lazy=10, generic=80, rust=0); 239 extern-C blocks audited
[3/121] gen cpp.rs (cppbind)
[4/121] gen JS modules (bundle-modules)
Preprocess modules (10027ms)
Bundle modules (55ms)
Postprocesss modules (28ms)
Bundle Functions (658ms)
Generate Code (23ms)

[10.80s] Bundled "src/js" for production
  2610 kb
  197 internal modules
  13 native modules
  91 internal functions across 17 files
[4/120] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu)

  nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19)

�[1m�[92m   Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core)
�[1m�[92m   Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno)
�[1m�[92m   Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr)
�[1m�[92m   Compiling
... (truncated)
diff hotspot
packages/bun-usockets/src/bsd.c                    |  38 ----
 packages/bun-usockets/src/context.c                |  20 --
 packages/bun-usockets/src/crypto/openssl.c         |  11 +-
 packages/bun-usockets/src/eventing/epoll_kqueue.c  |   4 -
 .../bun-usockets/src/internal/networking/bsd.h     |   1 -
 packages/bun-usockets/src/libusockets.h            |  21 --
 packages/bun-usockets/src/loop.c                   |   4 -
 packages/bun-usockets/src/node_quic_shim.c         |   1 -
 packages/bun-usockets/src/quic.c                   |  27 +--
 packages/bun-usockets/src/quic.h                   |   9 +-
 packages/bun-usockets/src/socket.c                 |  68 -------
 packages/bun-usockets/src/udp.c                    |   4 -
 src/bun_core/env_var.rs                            |   1 -
 src/http/h3_client/ClientContext.rs                |   6 +-
 src/jsc/bindings/ErrorStackTrace.cpp               |  15 --
 src/jsc/bindings/ErrorStackTrace.h                 |   4 -
 src/jsc/bindings/InspectorHTTPServerAgent.cpp      |  40 ----
 src/jsc/bindings/InspectorHTTPServerAgent.h        |   5 -
 src/jsc/bindings/JSBuffer.cpp                      |  45 -----
 src/jsc/bindings/JSDOMExceptionHandling.cpp        |   5 -
 src/jsc/bindings/JSDOMExceptionHandling.h          |   1 -
 src/jsc/bindings/ScriptExecutionContext.cpp        |  19 --
 src/jsc/bindings/ScriptExecutionContext.h          |   3 -
 src/jsc/bindings/ncrypto.cpp                       | 222 ---------------------
 src/jsc/bindings/ncrypto.h                         |  37 ----
 src/jsc/bindings/node/crypto/CryptoUtil.cpp        | 172 ----------------
 src/jsc/bindings/node/crypto/CryptoUtil.h          |   8 -
 src/jsc/bindings/node/http/llhttp/api.c            | 140 -------------
 src/jsc/bindings/node/http/llhttp/llhttp.h         |  46 -----
 src/jsc/bindings/webcore/EventEmitter.cpp          |   1 -
 src/jsc/bindings/webcore/EventTarget.cpp           |   2 -
 src/jsc/bindings/webcore/JSAbortAlgorithm.cpp
... (truncated)

gate history · 1 passed · 1 rejected · iteration 1

evidence per changed file
file                                                 reads  edits  tests
packages/bun-usockets/src/bsd.c                          0      0      0
packages/bun-usockets/src/context.c                      0      0      0
packages/bun-usockets/src/crypto/openssl.c               0      0      0
packages/bun-usockets/src/eventing/epoll_kqueue.c        0      0      0
packages/bun-usockets/src/internal/networking/bsd.h      0      0      0
packages/bun-usockets/src/libusockets.h                  0      0      0
packages/bun-usockets/src/loop.c                         0      0      0
packages/bun-usockets/src/node_quic_shim.c               0      0      0
packages/bun-usockets/src/quic.c                         0      0      0
packages/bun-usockets/src/quic.h                         0      0      0
packages/bun-usockets/src/socket.c                       0      0      0
packages/bun-usockets/src/udp.c                          0      0      0
src/bun_core/env_var.rs                                  2      2      0
src/http/h3_client/ClientContext.rs                      0      0      0
src/jsc/bindings/ErrorStackTrace.cpp                     0      0      0
src/jsc/bindings/ErrorStackTrace.h                       0      0      0
(+ 34 more files)

…ckets, and llhttp

Relinking the debug binary with --gc-sections --print-gc-sections lists
every function nothing references at link time. The ones below also have
no textual references outside their own declaration and definition and
are not removed by any open PR, so they are deleted together with their
declarations and Rust-side FFI wrappers.

node:crypto: keyFromString, passphraseFromBufferSource, the header
overloads of parseKeyFormat/parseKeyType, ByteSource::fromBIO/foreign,
Rsa::encrypt/decrypt with the RSA_Cipher template and setRsaMgf1Md,
Cipher::ForEach and its helpers, checkScryptParams, scrypt, pbkdf2,
X509View::ifRsa/ifEc, BignumPointer::isOne, BIOPointer::NewFp.

JSC/WebCore bindings: the per-request InspectorHTTPServerAgent
notifications, ScriptExecutionContext::ensureOnMainThread and
executionContext(), JSCStackTrace::getStackTraceForThrownValue,
JSCStackFrame::typeName, PerformanceResourceTiming::create and the
helpers only it used, rejectPromiseWithGetterTypeError and the
never-instantiated RejectPromise branch of IDLAttribute::get,
JSErrorHandler (whole files), the callback-interface toJS overloads, and
the JSBuffer DOMJIT alloc operations whose signatures were removed in
#9457.

uSockets: unused accessors (us_socket_is_tls, us_socket_detach,
us_socket_pair, us_socket_open, us_listen_socket_ext/port/get_fd,
us_socket_group_next/timestamp, us_loop_iteration_number, us_poll_ext,
us_connecting_socket_get_loop, the UDP local-ip accessor, several QUIC
accessors and the never-set on_open callback) and their Rust wrappers.

llhttp: the __wasm__-only block and the getter/name/reset/settings_init
API that the HTTP parser bindings never call.

Rust: four env vars whose readers were not carried over from the Zig
sources, and unused re-exports in node_fs.

A source lint pins every removed symbol.
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Removed unused public APIs, internal helpers, FFI bindings, and implementations across uSockets, QUIC, crypto, JSC, WebCore, llhttp, filesystem, and environment-variable code. Added source-lint checks to prevent removed symbols from returning.

Changes

Dead API cleanup

Layer / File(s) Summary
uSockets and QUIC API removal
packages/bun-usockets/src/*
Removed obsolete socket, listener, polling, UDP, SNI, loop, and QUIC APIs. QUIC pending connections no longer store caller userdata.
Rust FFI and wrapper alignment
src/http/h3_client/ClientContext.rs, src/lsquic_sys/lib.rs, src/uws_sys/*
Updated HTTP/3 and Rust wrappers for removed uSockets and QUIC bindings.
Crypto API cleanup
src/jsc/bindings/ncrypto.*, src/jsc/bindings/node/crypto/CryptoUtil.*
Removed unused ncrypto operations, key helpers, cipher enumeration, parsing helpers, and ByteSource factories.
JSC and WebCore symbol removal
src/jsc/bindings/ErrorStackTrace.*, src/jsc/bindings/InspectorHTTPServerAgent.*, src/jsc/bindings/JSBuffer.cpp, src/jsc/bindings/webcore/*
Removed unused stack-frame, inspector, buffer, script-context, callback-conversion, DOM, event-handler, and resource-timing APIs.
Parser, filesystem, and environment visibility cleanup
src/jsc/bindings/node/http/llhttp/*, src/runtime/node/node_fs.rs, src/bun_core/env_var.rs
Removed unused llhttp APIs and the crash-state feature flag. Restricted filesystem helper types to crate visibility.
Dead-symbol regression checks
test/internal/source-lints/dead-symbols-linker-sweep.test.ts
Added source-tree checks for removed symbols, deleted files, FFI APIs, llhttp markers, and the environment variable.

Possibly related PRs

  • oven-sh/bun#36576: Removes overlapping dead APIs from C++ and Rust bindings.
  • oven-sh/bun#37149: Shares cleanup areas in ncrypto, JSBuffer, ScriptExecutionContext, and dead-symbol tests.
  • oven-sh/bun#37332: Removes unused bindings and adds guards against symbol reintroduction.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: removing dead code from the listed bindings and libraries.
Description check ✅ Passed The description explains the scope, discovery method, verification, and behavior impact, although it does not use the template headings verbatim.

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

Status (head f0bbd11, plus an empty re-run commit c75475c): ready for a maintainer once build 92131 finishes.

  • Automated reviews are done: claude[bot] found no bugs on the first head and three nits across the later ones (a stale on_open doc mention, the write-only QUIC pending-connect user plumbing, a stale "too" in the SNI comment), all fixed in 2877209 and f0bbd11 with the threads resolved, and its pass on f0bbd11 found nothing further. CodeRabbit's review has no findings inside the diff; its one outside-diff note (SNI userdata keyed by SSL_CTX) describes pre-existing behavior this PR does not change and is answered below. No open threads.
  • The diff is -1302/+16 in src and packages plus the source lint (the additions are import, signature and comment adjustments). 531dbe1 dropped the two deletions the duplicate-detection bot found in other open PRs (three env vars in Remove dead code from C++ bindings, src/js builtins, CSS, and Rust util crates #35437, getStackTraceForThrownValue in Error.prepareStackTrace: index source URLs by visible frame, not by JSC frame #37450); a line-level re-check against every open PR touching these files found nothing else shared, and no open PR adds a caller of anything removed here (usockets: rewrite in Rust #33933 ports the same unused uSockets accessors 1:1).
  • Verification: every removed function was discarded by the linker (relink with --gc-sections --print-gc-sections) and has no textual reference outside its own declaration/definition. bun bd is green, cargo check --workspace passed on all 10 CI triples before the last two commits and the crates they touch re-checked clean on the host, Windows and macOS triples, fmt and clang-format are clean, the test files listed in the description pass (plus fetch-http3-client.test.ts for f0bbd11), and the lint fails on a tree without these deletions and passes here. CI: the first head passed outright (build 92078, 190/190); build 92122 for f0bbd11 was green except for test/cli/install/bun-install-registry.test.ts on the Windows 11 aarch64 lane, which this PR does not touch (reported for main-break triage), so it was re-run once as build 92131.
  • The only hunk that needs a human call is the BUN_DUMP_STATE_ON_CRASH declaration in env_var.rs: its reader lived in the Zig sources and was not ported, so nothing has read it since May. If the feature should come back instead, say so and I will drop that line; nothing else depends on it. Everything else is a leftover with no callers.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and found no bugs. Given the breadth — ~1,300 lines removed across 48 files spanning node:crypto, JSC/WebCore bindings, uSockets/QUIC, and llhttp — a human skim would still be worthwhile before merging.

What was reviewed:

  • Spot-checked removed uSockets symbols (us_socket_detach, us_poll_ext, us_socket_pair, us_listen_socket_get_fd, us_loop_iteration_number, SNI userdata, QUIC on_open) against src/, packages/bun-uws, and the libuv backend — no remaining callers; sni_node_t::user removal is safe since user is still stashed on the SSL_CTX ex_data.
  • Verified parseKeyFormat/parseKeyType ThrowScope overloads remain; CastedThisErrorBehavior::RejectPromise enum value stays for JSDOMOperationReturningPromise and no IDLAttribute::get<..., RejectPromise> instantiation exists.
  • Confirmed JSErrorHandler has zero remaining references and us_internal_ssl_detach is still live via us_internal_socket_close_raw.
Extended reasoning...

Overview

This PR removes ~1,332 lines of provably dead code across 48 files: node:crypto helpers (CryptoUtil.cpp, ncrypto.cpp), JSC/WebCore bindings (ErrorStackTrace, InspectorHTTPServerAgent, ScriptExecutionContext, JSBuffer, JSDOMAttribute, JSErrorHandler, JSAbortAlgorithm, JSPerformanceObserverCallback, PerformanceResourceTiming), the in-tree uSockets fork (TCP/UDP/QUIC accessors, SNI userdata field), llhttp API surface, and Rust FFI wrappers plus unused env-var declarations. Candidates were found via --gc-sections --print-gc-sections on the debug binary, then filtered by textual reference across all source trees and open-PR diffs. A new source-lint test pins every removed symbol.

Security risks

None introduced. Removals touch crypto (ncrypto, CryptoUtil) and TLS SNI (sni_node_t::user), but only delete unreferenced helpers; the live paths (Cipher::encrypt/decrypt, ThrowScope-taking parseKeyFormat/parseKeyType, SSL_CTX ex_data for SNI userdata) are untouched. I verified user is still stored via SSL_CTX_set_ex_data in us_listen_socket_add_server_name, so us_socket_server_name_userdata continues to work.

Level of scrutiny

High, warranting human review. While each individual removal is mechanical and the methodology (linker-driven, cross-referenced against codegen output, all 10 target triples checked) is sound, the aggregate touches critical paths: crypto primitives, JSC GC-sensitive callback wrappers, QUIC connection lifecycle (on_open field/dispatch removed), TLS SNI storage layout, and HTTP parser API. A maintainer familiar with the in-flight work referenced in the description (#33933 uSockets rewrite, #37101, #37149) should confirm none of these are about to gain callers.

Other factors

  • Spot-checks against src/, packages/, and both eventing backends confirm zero remaining callers for a representative sample of removed symbols.
  • The CastedThisErrorBehavior::RejectPromise branch removal from IDLAttribute::get is safe: the enum value remains for JSDOMOperationReturningPromise, and grep confirms no IDLAttribute::get<..., RejectPromise> instantiation exists.
  • Only stale references remain in packages/bun-usockets/misc/manual.md (non-compiled documentation).
  • The new source-lint test reads HEAD via git show (matching the sibling dead-code-escapes.test.ts pattern) and its regexes correctly use word boundaries so e.g. \\bus_quic_socket_context\\b won't false-match us_quic_socket_context_t.
  • PR description states bun bd and rust:check-all pass on all targets and lists the test suites run; the test file follows the test/internal/source-lints/ convention.

@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for the pass. On the two points it leaves for a human:

  • In-flight work gaining callers: I checked the diffs of the open PRs that touch these areas (the ~60 open crypto/tls/x509 PRs and the ~80 open http/socket/udp/quic/inspector/performance PRs) for any added reference to a removed symbol. The only hit is usockets: rewrite in Rust #33933, the uSockets-in-Rust rewrite, which ports the same never-called accessors 1:1 as part of porting the whole C surface; nothing else adds a caller. usockets(kqueue): deliver a peer reset to sockets polling for no events #37101 only names us_socket_detach in a comment, and Remove dead code from C++ bindings, bindgen glue, ast, and orphaned scripts #37149 removes different functions in ncrypto.* (enumUsages, NewFile), so the adjacent hunks there are a textual rebase at most. The on_open removal is behavior-neutral: both QUIC contexts are us_calloc'd and nothing ever called the setter, so the dispatch on connection open was always a null check.
  • packages/bun-usockets/misc/manual.md still lists us_loop_iteration_number and us_poll_ext. That file is the upstream manual and already documents the us_socket_context API that this fork replaced a while ago, so I left it alone rather than make it look current; happy to drop the two lines if a maintainer prefers.

No open threads. CI build 92078 is in progress (150 jobs green so far, none failed).

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. Remove dead code from C++ bindings, src/js builtins, CSS, and Rust util crates #35437 - removes the byte-identical BUN_NEEDS_PROC_SELF_WORKAROUND, MI_VERBOSE, and TODIUM declarations from src/bun_core/env_var.rs as part of the same dead-symbol sweep.
  2. Error.prepareStackTrace: index source URLs by visible frame, not by JSC frame #37450 - deletes the same dead JSCStackTrace::getStackTraceForThrownValue definition and declaration from ErrorStackTrace.cpp/ErrorStackTrace.h.

🤖 Generated with Claude Code

@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 2:49 AM PT - Aug 11th, 2026

@robobun, your commit c75475c6803c652797e4341766e3387eb686d399 passed in Build #92131! 🎉


🧪   To try this PR locally:

bunx bun-pr 37454

That installs a local version of the PR into your bun-37454 executable, so you can run:

bun-37454 --bun

…7450 already remove

BUN_NEEDS_PROC_SELF_WORKAROUND, MI_VERBOSE and TODIUM are removed by
#35437, and JSCStackTrace::getStackTraceForThrownValue by #37450, so this
PR keeps only the deletions that are not already open elsewhere.
@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

The duplicate-detection bot is right about both overlaps, and they are out of this PR as of 531dbe1:

I also re-diffed the removed lines of this PR against every open PR that touches any of these files (including #37325, #37420, #37427, #37447, #37450 and #37452, which were opened after my first check): there are no other identical deletions, so the rest of the PR is not a duplicate of anything open. The lint and the description are updated to match; the diff is now -1288/+9 in src and packages. CodeRabbit is still rate-limited on this PR, so there is no review from it yet; CI for the new head is build 92093.

Comment thread src/uws_sys/quic/Socket.rs
@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 2877209 for the one claude[bot] finding on 531dbe1 (a doc comment in src/uws_sys/quic/Socket.rs still mentioned the removed QUIC on_open callback); the thread is answered and resolved, and nothing else in the tree mentions the removed names outside the upstream manual.md noted earlier. The change is comment-only, so the code is identical to 531dbe1, whose build (92093) had 160 jobs green and no failures when this push superseded it; build 92103 is running for the new head. CodeRabbit is still rate-limited on this PR, so there is nothing from it to address.

Comment thread packages/bun-usockets/src/quic.h Outdated
Comment thread packages/bun-usockets/src/crypto/openssl.c Outdated
Removing us_quic_pending_connect_user() left us_quic_pending_connect_s::user
write-only, so the field, the void *user parameter of
us_quic_socket_context_connect, and the argument the h3 client threaded
through Context::connect go too. The SSL_CTX ex_data is now the only place
SNI userdata is stashed, so its comment no longer says "too".
@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed f0bbd11 for the two claude[bot] findings on 2877209: the pending-connect user plumbing that us_quic_pending_connect_user was the last reader of is removed end to end (field, void *user parameter of us_quic_socket_context_connect, the Context::connect argument and the h3 client call site), and the SNI comment no longer says the userdata is stashed on the SSL_CTX "too". Both threads are answered and resolved. 00c7d99, just before it, only changes the new lint so its symbol checks read the working tree instead of HEAD. cargo check is clean on the host, Windows and macOS triples, bun bd builds, fetch-http3-client.test.ts passes, and the diff is now -1302/+16 in src and packages; the description is updated. CI for this head is build 92122. CodeRabbit is still rate-limited, so nothing from it to address.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both earlier findings are addressed in f0bbd11 (the write-only us_quic_pending_connect_s::user field/parameter chain is gone through to ClientContext::connect, and the stale "too" in the openssl.c SNI comment is dropped); nothing new turned up on this head. Given the breadth (50 files across crypto, JSC/WebCore bindings, uSockets/QUIC, llhttp) and the flagged maintainer call on whether BUN_DUMP_STATE_ON_CRASH should be reimplemented rather than removed, a human look is still warranted.

Checked on this head: the \bus_quic_socket_context\b lint regex does not false-positive on us_quic_socket_context_t/_connect/_on_* (underscore is a word char, no boundary); the updated Context::connect extern signature matches the C definition after the void *user drop; CastedThisErrorBehavior::RejectPromise has no remaining users after its IDLAttribute::get branch was removed.

Extended reasoning...

Overview

Fourth pass on this PR. The head moved from 2877209 to f0bbd11 since my last review; that commit addresses both inline findings I left at 09:00 (the write-only us_quic_pending_connect_s::user field and its parameter chain through quic.h/quic.c/Context.rs/ClientContext.rs, and the dangling "too" in the openssl.c SNI-userdata comment). Commit 00c7d99 also switched the source-lint's symbol checks to read the working tree rather than HEAD, which is orthogonal. The bug-hunting pass on f0bbd11 found nothing.

Security risks

None introduced. Everything removed was already discarded by the linker (--gc-sections --print-gc-sections), so the shipped binary is byte-identical modulo section ordering. The one non-deletion behavior touch — dropping the void *user parameter from us_quic_socket_context_connect — is an FFI signature change I introduced via review; I verified the C definition, header, Rust extern, wrapper, and sole caller all agree on the new arity.

Level of scrutiny

High, because of breadth: 50 files across node:crypto (ncrypto.*, CryptoUtil.*), JSC/WebCore bindings (Buffer DOMJIT ops, JSErrorHandler, IDLAttribute, ScriptExecutionContext, PerformanceResourceTiming), the in-tree uSockets fork (TCP/UDP/QUIC/SNI), llhttp, and Rust FFI wrappers. Each individual hunk is a mechanical deletion, but the interaction with ~open PRs and the one flagged product decision (BUN_DUMP_STATE_ON_CRASH — its reader was never ported from Zig, so removing the declaration vs. reimplementing the feature is a maintainer call the author explicitly deferred) push this past the auto-approve bar.

Other factors

The author's own status comment says "ready for a maintainer once build 92103 finishes" and flags the env-var hunk as needing a human call. CI on the code-identical predecessor 531dbe1 was 160/160 green when superseded; f0bbd11's build 92122 is in flight. All three of my prior findings across earlier heads have been fixed; there are no open threads I'm aware of beyond the two 09:00 inline comments this head resolves.

@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks, agreed on the human look: the current state is summarized in the status comment at the top (head f0bbd11, no open threads, build 92122 in progress), and the one item that is a judgment call rather than a leftover is the BUN_DUMP_STATE_ON_CRASH declaration, which I will drop from the PR on request if reimplementing it is preferred. Nothing else to change from this pass.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/bun-usockets/src/crypto/openssl.c (1)

2960-2969: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Store SNI userdata per registration.

user is documented as per-domain data, but this code stores it in one SSL_CTX slot. If one context serves multiple hostnames, the last registration makes every hostname use the same HttpRouter. A duplicate registration can also overwrite the slot before sni_add rejects it. Store user with the SNI entry, or enforce a one-context/one-user invariant before the write.

🤖 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 `@packages/bun-usockets/src/crypto/openssl.c` around lines 2960 - 2969, Update
the SNI registration flow around sni_add so the per-domain user value is stored
on each SNI node/entry rather than unconditionally in the shared SSL_CTX ex-data
slot. Ensure duplicate hostname registration cannot overwrite existing userdata
before sni_add rejects it, while preserving the existing rollback via
sni_node_destructor and return value.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@packages/bun-usockets/src/crypto/openssl.c`:
- Around line 2960-2969: Update the SNI registration flow around sni_add so the
per-domain user value is stored on each SNI node/entry rather than
unconditionally in the shared SSL_CTX ex-data slot. Ensure duplicate hostname
registration cannot overwrite existing userdata before sni_add rejects it, while
preserving the existing rollback via sni_node_destructor and return value.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ec504994-1266-400e-9a6a-bf0e8116a5e9

📥 Commits

Reviewing files that changed from the base of the PR and between 54d6d16 and c75475c.

📒 Files selected for processing (50)
  • packages/bun-usockets/src/bsd.c
  • packages/bun-usockets/src/context.c
  • packages/bun-usockets/src/crypto/openssl.c
  • packages/bun-usockets/src/eventing/epoll_kqueue.c
  • packages/bun-usockets/src/internal/networking/bsd.h
  • packages/bun-usockets/src/libusockets.h
  • packages/bun-usockets/src/loop.c
  • packages/bun-usockets/src/node_quic_shim.c
  • packages/bun-usockets/src/quic.c
  • packages/bun-usockets/src/quic.h
  • packages/bun-usockets/src/socket.c
  • packages/bun-usockets/src/udp.c
  • src/bun_core/env_var.rs
  • src/http/h3_client/ClientContext.rs
  • src/jsc/bindings/ErrorStackTrace.cpp
  • src/jsc/bindings/ErrorStackTrace.h
  • src/jsc/bindings/InspectorHTTPServerAgent.cpp
  • src/jsc/bindings/InspectorHTTPServerAgent.h
  • src/jsc/bindings/JSBuffer.cpp
  • src/jsc/bindings/JSDOMExceptionHandling.cpp
  • src/jsc/bindings/JSDOMExceptionHandling.h
  • src/jsc/bindings/ScriptExecutionContext.cpp
  • src/jsc/bindings/ScriptExecutionContext.h
  • src/jsc/bindings/ncrypto.cpp
  • src/jsc/bindings/ncrypto.h
  • src/jsc/bindings/node/crypto/CryptoUtil.cpp
  • src/jsc/bindings/node/crypto/CryptoUtil.h
  • src/jsc/bindings/node/http/llhttp/api.c
  • src/jsc/bindings/node/http/llhttp/llhttp.h
  • src/jsc/bindings/webcore/EventEmitter.cpp
  • src/jsc/bindings/webcore/EventTarget.cpp
  • src/jsc/bindings/webcore/JSAbortAlgorithm.cpp
  • src/jsc/bindings/webcore/JSAbortAlgorithm.h
  • src/jsc/bindings/webcore/JSDOMAttribute.h
  • src/jsc/bindings/webcore/JSErrorHandler.cpp
  • src/jsc/bindings/webcore/JSErrorHandler.h
  • src/jsc/bindings/webcore/JSPerformanceObserverCallback.cpp
  • src/jsc/bindings/webcore/JSPerformanceObserverCallback.h
  • src/jsc/bindings/webcore/PerformanceResourceTiming.cpp
  • src/jsc/bindings/webcore/PerformanceResourceTiming.h
  • src/jsc/bindings/webcore/ResourceTiming.cpp
  • src/jsc/bindings/webcore/ResourceTiming.h
  • src/lsquic_sys/lib.rs
  • src/runtime/node/node_fs.rs
  • src/uws_sys/ListenSocket.rs
  • src/uws_sys/SocketGroup.rs
  • src/uws_sys/quic/Context.rs
  • src/uws_sys/quic/Socket.rs
  • src/uws_sys/us_socket_t.rs
  • test/internal/source-lints/dead-symbols-linker-sweep.test.ts
💤 Files with no reviewable changes (41)
  • src/jsc/bindings/webcore/JSDOMAttribute.h
  • packages/bun-usockets/src/udp.c
  • src/bun_core/env_var.rs
  • src/jsc/bindings/webcore/ResourceTiming.h
  • src/jsc/bindings/webcore/EventEmitter.cpp
  • src/jsc/bindings/webcore/JSAbortAlgorithm.cpp
  • src/jsc/bindings/InspectorHTTPServerAgent.h
  • packages/bun-usockets/src/eventing/epoll_kqueue.c
  • src/jsc/bindings/webcore/PerformanceResourceTiming.h
  • packages/bun-usockets/src/internal/networking/bsd.h
  • src/jsc/bindings/webcore/JSAbortAlgorithm.h
  • src/jsc/bindings/JSDOMExceptionHandling.cpp
  • src/jsc/bindings/webcore/JSErrorHandler.h
  • packages/bun-usockets/src/loop.c
  • src/lsquic_sys/lib.rs
  • src/jsc/bindings/JSDOMExceptionHandling.h
  • src/uws_sys/us_socket_t.rs
  • src/jsc/bindings/node/http/llhttp/llhttp.h
  • src/jsc/bindings/webcore/JSErrorHandler.cpp
  • packages/bun-usockets/src/node_quic_shim.c
  • packages/bun-usockets/src/bsd.c
  • packages/bun-usockets/src/socket.c
  • src/jsc/bindings/webcore/ResourceTiming.cpp
  • src/jsc/bindings/webcore/EventTarget.cpp
  • src/jsc/bindings/ScriptExecutionContext.cpp
  • packages/bun-usockets/src/context.c
  • src/jsc/bindings/ErrorStackTrace.h
  • src/jsc/bindings/webcore/PerformanceResourceTiming.cpp
  • src/jsc/bindings/ErrorStackTrace.cpp
  • src/jsc/bindings/webcore/JSPerformanceObserverCallback.h
  • src/jsc/bindings/node/crypto/CryptoUtil.cpp
  • src/jsc/bindings/node/crypto/CryptoUtil.h
  • src/uws_sys/SocketGroup.rs
  • src/jsc/bindings/ncrypto.h
  • src/jsc/bindings/JSBuffer.cpp
  • src/jsc/bindings/ncrypto.cpp
  • packages/bun-usockets/src/libusockets.h
  • src/jsc/bindings/node/http/llhttp/api.c
  • src/jsc/bindings/webcore/JSPerformanceObserverCallback.cpp
  • src/jsc/bindings/ScriptExecutionContext.h
  • src/jsc/bindings/InspectorHTTPServerAgent.cpp

@robobun

robobun commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

On CodeRabbit's one note (the SNI userdata living in the SSL_CTX ex_data slot, openssl.c around line 2960): that is how main already works and this PR does not change it. The per-node sni_node_t::user copy removed here was never read by anything (us_listen_socket_find_server_name_userdata, its only reader, had no callers), and us_socket_server_name_userdata has been reading the ex_data slot all along, so the lookup behavior before and after this PR is identical. The shared-context scenario also cannot arise with the current callers: TemplatedApp::addServerName (packages/bun-uws/src/App.h) builds a fresh SSL_CTX and a fresh HttpRouter for every hostname pattern and registers them together, so the context-to-userdata mapping is 1:1 by construction, and Bun.listen (src/runtime/socket/Listener.rs) always passes a null userdata. A duplicate registration writes the ex_data of the new, about-to-be-freed context, not the existing one. Nothing to change here; a per-entry lookup would be a new feature with no consumer rather than dead-code removal.

No open threads. Build 92131 (the re-run after an unrelated bun-install-registry failure on the Windows aarch64 lane of 92122) is at 180/190 with no failures so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants