Ban <iostream> from release builds and drop it from bun-uws - #35256
Conversation
A single #include <iostream> anywhere in the release link pulls libstdc++'s globals_io.o in: its _GLOBAL__sub_I.00090_globals_io.cc static initializer constructs cin/cout/cerr/clog (char and wchar_t) and references the full std::locale facet set, so roughly fifty libstdc++ functions (std::ios_base::Init, std::locale::_S_initialize, std::locale::_Impl, ctype/numpunct/moneypunct/timepunct/messages) run before main on every bun process. There were two sources of the include: the vendored simdutf header in WebKit (Source/WTF/wtf/simdutf/simdutf_impl.h, fixed in oven-sh/WebKit#320) and five bun-uws headers. Three of the bun-uws headers (AsyncSocket.h, HttpRouter.h, Loop.h) included it without using it; App.h, HttpContext.h and TopicTree.h wrote fixed error strings via std::cerr, now fputs(stderr). The new src/banned-includes/iostream shim is prepended to the -I path for release builds only, so any #include <iostream> resolves to a #error with a pointer to this explanation. This also catches WebKit headers that start including it (SIMDUTF.h etc. are pulled into Bun TUs via helpers.h), so a future WebKit bump that regresses this fails the release compile rather than silently re-adding the initializers. Debug builds keep the real header for ad-hoc printf debugging; -DBUN_ALLOW_IOSTREAM is the opt-out. With both this change and the WebKit patch applied: nm build/release/bun-profile | grep ios_base4Init -> empty _GLOBAL__sub_I.00090_globals_io.cc gone std::locale::_S_initialize / _Impl ctor gone bun-profile: 547 KB smaller The four remaining _GLOBAL__sub_I_*locale_inst.cc stubs are facet-id guard-byte writes (a few dozen movb $1 each) pulled by Int128.cpp's std::ostringstream; those are near-free and <sstream> does not emit the static Init object.
|
Updated 10:05 PM PT - Jul 23rd, 2026
@Jarred-Sumner, your commit df60661 is building: |
Picks up oven-sh/WebKit#320 (removes the stray <iostream> from the vendored simdutf amalgamation and bans <iostream> at compile time for non-Debug USE_BUN_JSC_ADDITIONS builds) and oven-sh/WebKit#322 (skip the eager timezone prewarm under USE_BUN_JSC_ADDITIONS). With this bump the release build's src/banned-includes/iostream shim no longer trips on the WebKit simdutf header, and no translation unit in the final link carries a reference to std::ios_base_library_init.
WalkthroughChangesThe PR bans unauthorized iostream enforcement
WebKit version pin
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
@robobun bump WebKit sha I mergedbmore WebKit PRs and this can close the other WebKit bumps |
Bun validates idleTimeout/maxLifetime and the request-handler contract before calling into uWS, so these paths are programmer errors rather than user-facing conditions; std::terminate is enough.
|
Bumped Also dropped the stderr logging at the five uWS validation sites per the review above; they now Heads up: the autobuild-af2e8dc6... release publish job is still uploading assets (Linux glibc variants are up; musl/macOS/Windows/FreeBSD/Android still pending), so those CI lanes will 404 on the prebuilt download until the upload finishes. |
…r call sites The stderr message they referred to was removed in the previous commit.
…scans nothing Matches the convention in the sibling lints (shim-stdint-includes, unsound-erased-box, frozen-nonnull-reborrow, expect-call-counter).
|
CI on build 78668: 190/196 jobs passed. Every The remaining red is unrelated to this diff:
Ten other tests (webview-chrome, node-dns, napi, fastutf8stream-reopen, http-agent-keepalive, solc, no-orphans, fetch.stream, 20144, complex-workspace) went yellow and passed on retry. Ready for review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@test/internal/source-lints/no-iostream-include.test.ts`:
- Around line 31-48: Update the scan in the test to track the number of matched
files separately for each configured root, and assert that every root in roots
has at least one scanned file. Replace the aggregate scanned > 0 guard with
per-root precondition assertions while preserving the existing violation
detection and allowlist behavior.
- Line 29: Replace the regex-based iostreamInclude check with preprocessor-aware
tokenization or the repository’s existing parser so comments and line-spliced
directives are recognized. Update the test coverage to include variants such as
`#/**/include <iostream>` and `#incl\\\nude <iostream>`, while preserving
detection of the standard spelling.
🪄 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
Run ID: 594e6012-3af3-45df-84fd-1da2321254e6
📒 Files selected for processing (10)
packages/bun-uws/src/App.hpackages/bun-uws/src/AsyncSocket.hpackages/bun-uws/src/HttpContext.hpackages/bun-uws/src/HttpRouter.hpackages/bun-uws/src/Loop.hpackages/bun-uws/src/TopicTree.hscripts/build/deps/webkit.tsscripts/build/flags.tssrc/banned-includes/iostreamtest/internal/source-lints/no-iostream-include.test.ts
💤 Files with no reviewable changes (7)
- packages/bun-uws/src/HttpRouter.h
- packages/bun-uws/src/Loop.h
- packages/bun-uws/src/AsyncSocket.h
- packages/bun-uws/src/HttpContext.h
- packages/bun-uws/src/App.h
- scripts/build/deps/webkit.ts
- packages/bun-uws/src/TopicTree.h
So the guard still trips if one of the three roots is renamed while the others still yield files.
This reverts commit 507ab81.
There was a problem hiding this comment.
LGTM — mechanical header cleanup plus a release-build enforcement shim, with the maintainer already having directed the shape of the changes.
What was reviewed:
- bun-uws header edits are pure deletions of
<iostream>includes and the fivestd::cerrmessages; thestd::terminate()calls (already present pre-PR) remain and compiled green on everybuild-cpplane in #78668. bunIncludes()prependssrc/banned-includesonly whencfg.release, so debug builds are unaffected;#include_nextin the shim is fine since Bun builds exclusively with clang.- Net
webkit.tschange vs main is the comment deletion only (per maintainer request);WEBKIT_VERSIONstays ata40d462206, which robobun verified is a descendant of the #320 simdutf fix. - The source-lint test now asserts
scanned > 0per root and matches the sibling lints' convention.
Extended reasoning...
Overview
Ten files: six bun-uws headers drop #include <iostream> and the std::cerr << ... lines that were the only consumers (leaving the existing std::terminate() calls); scripts/build/flags.ts prepends src/banned-includes to the release -I list; src/banned-includes/iostream is a new #error shim with a BUN_ALLOW_IOSTREAM / #include_next opt-out; test/internal/source-lints/no-iostream-include.test.ts is a new debug-CI backstop; and scripts/build/deps/webkit.ts — after the main merge — nets out to deleting the 7-line why-comment above WEBKIT_VERSION (the sha itself is unchanged from main).
Security risks
None. No user-facing input handling, no auth/crypto, no runtime code paths — only build-system include ordering, vendored-header include hygiene, and a source-lint test. The deleted std::cerr messages sat on unreachable-in-Bun validation branches (Bun validates idleTimeout/maxLifetime etc. before calling into uWS) that already std::terminate()d.
Level of scrutiny
Low-to-moderate. The bun-uws edits are pure deletions verified by cross-platform compile (build #78668: every build-cpp/build-bun lane green across Linux glibc/musl, macOS, Windows, FreeBSD, Android). The flags.ts change is a single conditional array spread in bunIncludes() following the file's existing pattern. The shim uses #include_next, a clang/GCC extension — fine because Bun only builds with clang (per flags.ts). The webkit.ts merge story was checked: my earlier concern that a40d462206 predated oven-sh/WebKit#320 was refuted by robobun's git merge-base --is-ancestor check, so the release shim will not trip on the WebKit prebuilt.
Other factors
The maintainer has actively steered this PR three times ("Just don't log anything. Delete it.", "bump WebKit sha", "No. The why comment is unnecessary.") and every one of those directions is reflected in the current diff. All five prior review threads (mine and CodeRabbit's) are resolved. The only post-#78668 changes are the per-root scanned > 0 guard in the lint test and a comment restore+revert in webkit.ts — neither affects compilation. Build #78967 for the current HEAD is in progress; given the delta from the last green build is test-only + comment-only, that is not a reason to hold.
|
Build 78967 (post-merge, |
There was a problem hiding this comment.
LGTM — mechanical removal of <iostream> from bun-uws with compile-time and source-lint enforcement; all prior review feedback (Jarred's, mine, CodeRabbit's) has been applied.
What was reviewed:
- Confirmed the five removed
std::cerrsites guard unreachable paths:idleTimeout/maxLifetimeare validated inWebSocketServerContext.rs(idleTimeout clamped to [8, 960]; maxLifetime is never read from JS and stays 0), and the HttpContext/TopicTree checks are Bun-internal programmer-error guards. - Verified
scripts/build/deps/webkit.tsis no longer in the diff after df60661 — main'sWEBKIT_VERSION = a40d462206already contains oven-sh/WebKit#320, so the shim won't trip on the simdutf header. - Checked the
bunIncludes()change prependssrc/banned-includesonly forcfg.release, matching the shim's stated debug-build opt-out. - The source-lint test now asserts
scanned > 0per root and matches the sibling lints' conventions.
Extended reasoning...
Overview
This PR eliminates <iostream> from Bun's release link so libstdc++'s globals_io.o (and its ~50-function locale facet initializer chain) no longer runs before main. It touches nine files: six bun-uws headers (drop the include and five std::cerr << ... << std::endl lines that preceded std::terminate()), scripts/build/flags.ts (prepend src/banned-includes to the release -I list), a new #error shim at src/banned-includes/iostream with a BUN_ALLOW_IOSTREAM / #include_next opt-out, and a new source-lint test. scripts/build/deps/webkit.ts dropped out of the diff after merging main — the current WEBKIT_VERSION on main already contains the simdutf fix (oven-sh/WebKit#320).
Security risks
None. No user-facing API surface, no auth/crypto/network parsing, no untrusted-input handling. The only behavioral change is that five programmer-error std::terminate() sites in vendored bun-uws no longer print an explanatory line to stderr first. I verified those sites are unreachable from Bun user input: WebSocketServerContext.rs:323-343 clamps idleTimeout to 0 or [8, 960] and throws on out-of-range values before calling into uWS; max_lifetime defaults to 0 and is never populated from the JS options object; the HttpContext.h and TopicTree.h checks guard invariants Bun's own request-handler and pub/sub glue must uphold. Jarred explicitly directed the deletion ("Just don't log anything. Delete it.").
Level of scrutiny
Low-to-medium. The bun-uws edits are pure deletions of unused includes and pre-terminate() prints. The build-system change is six lines adding a release-only -I entry; the shim it points at fails compilation loudly rather than silently changing behavior, and #include_next preserves an escape hatch. The lint test is a debug-CI backstop for the same property. Every build-cpp/build-bun lane on build 78967 (post-merge, post-shim) is green across Linux glibc/musl, macOS x64/arm64, Windows x64/arm64, FreeBSD, and Android, so the shim demonstrably compiles cleanly against the pinned WebKit on every platform.
Other factors
This PR has been through four review rounds with the maintainer directly engaged. Jarred gave two explicit directions (delete the stderr logging; drop the WEBKIT_VERSION why-comment) — both applied. My three prior inline findings (missing <cstdio> IWYU → mooted by the logging deletion; stale "Notify user" comments; vacuous-scan guard in the lint test) were all addressed and resolved. CodeRabbit's per-root scan guard was applied; its preprocessor-tokenization suggestion was correctly declined (the release #error shim is the authoritative enforcement and is preprocessor-aware). The one merge-conflict finding I raised on webkit.ts was half-right (the dropped comment) and half-wrong (the sha ancestry) — robobun verified via git merge-base --is-ancestor that a40d462206 already contains #320, and the file has since converged with main. The remaining CI red on 78967 is the fleet-wide spawn.test.ts flake unrelated to this diff.
… is generated; already on main via #35256)
2 participants
Problem
Every Bun process runs libstdc++'s iostream and locale static initializers before
main. From the releasebun-profile:std::ios_base::Init,std::locale::_S_initialize,std::locale::_Impl, and construction of ctype/numpunct/moneypunct/timepunct/messages for bothcharandwchar_t: roughly fifty functions out of libstdc++ that sit ahead ofmainin thebun run orderfiletrace. Bun never uses C++ iostreams.Cause
On libstdc++,
<iostream>(and only<iostream>; not<ostream>/<istream>/<sstream>) emits an undefined reference to_ZSt21ios_base_library_initvin every TU that includes it. One such reference anywhere in the link pullsglobals_io.ofrom libstdc++.a, whose initializer constructscin/cout/cerr/clogand theirwchar_tsiblings, which in turn reference the full locale facet set.There are two sources:
Source/WTF/wtf/simdutf/simdutf_impl.h:9949, an unused include in thescalar/base64.hsection). BecauseSIMDUTF.his included fromsrc/jsc/bindings/helpers.h, ~70 Bun TUs carry the reference. Fixed in simdutf: remove unused <iostream> from scalar/base64.h amalgamation WebKit#320; upstream simdutf already dropped it in simpler approach to removing the C++ lib dependency at runtime simdutf/simdutf#962.AsyncSocket.h,HttpRouter.h,Loop.hinclude it without using it;App.h,HttpContext.h,TopicTree.hwrite fixed error strings viastd::cerr. 7 Bun TUs carry the reference via<bun-uws/src/App.h>.Changes
scripts/build/deps/webkit.ts: bumpWEBKIT_VERSIONtoaf2e8dc639(simdutf: remove unused <iostream> from scalar/base64.h amalgamation WebKit#320: drop the simdutf<iostream>include and ban it at compile time for non-DebugUSE_BUN_JSC_ADDITIONSbuilds; also picks up Make the WebAssembly namespace object lazy WebKit#321 lazy WebAssembly namespace and VM: skip eager timezone prewarm under USE(BUN_JSC_ADDITIONS) WebKit#322 skip eager timezone prewarm). Subsumes theWEBKIT_VERSIONchange in WebKit: skip eager timezone prewarm in VM::VM (1.3.14 startup regression) #35258.packages/bun-uws: drop the<iostream>include fromAsyncSocket.h,HttpContext.h,HttpRouter.h,Loop.h,TopicTree.h; delete the fivestd::cerr << ...validation messages (Bun validates these inputs before calling into uWS, so the paths are unreachable programmer errors andstd::terminate()alone is enough).src/banned-includes/iostream: a#errorshim prepended to the-Isearch path for release builds. Any#include <iostream>in a Bun TU (including transitively from a WebKit header) fails the release compile with an explanation pointing here. Debug builds keep the real header for ad-hoc printf debugging;-DBUN_ALLOW_IOSTREAMis the opt-out.test/internal/source-lints/no-iostream-include.test.ts: scanssrc/,packages/bun-uws,packages/bun-usocketsfor the include so debug CI also catches it.Verification
With both oven-sh/WebKit#320 and this change applied to a release build:
Four
_GLOBAL__sub_I_*locale_inst.ccstubs remain: these are the libstdc++ facet-id guard-byte initializers (a few dozenmovb $1,(%rax)each) pulled byInt128.cpp'sstd::ostringstream.<sstream>does not emit the staticInitobject and those stubs are near-free.Int128.cpp
Source/WTF/wtf/Int128.cppincludes<sstream>and<ostream>. Both are load-bearing (UInt128ToFormattedString'sstd::ostringstreamand theoperator<<(std::ostream&, ...)overloads) and neither emits the staticios_base::Initobject, so they are left alone.[decide:webkit] gate passed · iteration 6 · 10 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 8 passed · 1 rejected · iteration 6
evidence per changed file