Fix 100% CPU usage with idle WebSocket connections on macOS (kqueue) #25475
Conversation
|
Updated 4:43 PM PT - Dec 11th, 2025
❌ @cirospaciari, your commit e16acf9 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 25475That installs a local version of the PR into your bun-25475 --bun |
cirospaciari
changed the title
WalkthroughThe changes modify kqueue-based event loop handling to use stricter filter matching and one-shot semantics for write events, add corresponding loop handling for write completion under KQUEUE, and introduce a test fixture to verify WebSocket idle behavior does not consume excessive CPU. Changes
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
Comment |
![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
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/eventing/epoll_kqueue.c (1)
302-303: Critical: Inconsistent fix - same bug remains inus_loop_run_bun_tick.Lines 231-232 in
us_loop_runwere correctly changed fromfilter & EVFILT_*tofilter == EVFILT_*, but lines 302-303 here inus_loop_run_bun_tickstill use the incorrect bitwise AND comparison. This will cause the same 100% CPU issue in code paths that useus_loop_run_bun_tick.int events = 0 - | ((filter & EVFILT_READ) ? LIBUS_SOCKET_READABLE : 0) - | ((filter & EVFILT_WRITE) ? LIBUS_SOCKET_WRITABLE : 0); + | ((filter == EVFILT_READ) ? LIBUS_SOCKET_READABLE : 0) + | ((filter == EVFILT_WRITE) ? LIBUS_SOCKET_WRITABLE : 0);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/bun-usockets/src/eventing/epoll_kqueue.c(2 hunks)packages/bun-usockets/src/loop.c(2 hunks)test/js/bun/http/bun-server.test.ts(1 hunks)test/js/bun/http/bun-websocket-cpu-fixture.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
test/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test,describe,expect) and import frombun:test
Usetest.eachand data-driven tests to reduce boilerplate when testing multiple similar cases
Files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testwith files that end in*.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always useport: 0to get a random port
Prefer concurrent tests over sequential tests usingtest.concurrentordescribe.concurrentwhen multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, usebunExeandbunEnvfromharnessto ensure the same build of Bun is used and debug logging is silenced
Use-eflag for single-file tests when spawning Bun processes
UsetempDir()from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, usePromise.withResolversto create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
UseBuffer.alloc(count, fill).toString()instead of'A'.repeat(count)to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Usedescribeblocks for grouping related tests
Always useawait usingorusingto ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Usedescribe.each()for parameterized tests
UsetoMatchSnapshot()for snapshot testing
UsebeforeAll(),afterEach(),beforeEach()for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup inafterEach()
Files:
test/js/bun/http/bun-server.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: For single-file tests in Bun test suite, prefer using-eflag overtempDir
For multi-file tests in Bun test suite, prefer usingtempDirandBun.spawn
Always useport: 0when spawning servers in tests - do not hardcode ports or use custom random port functions
UsenormalizeBunSnapshotto normalize snapshot output in tests instead of manual output comparison
Never write tests that check for no 'panic', 'uncaught exception', or similar strings in test output - that is not a valid test
UsetempDirfromharnessto create temporary directories in tests - do not usetmpdirSyncorfs.mkdtempSync
In tests, callexpect(stdout).toBe(...)beforeexpect(exitCode).toBe(0)when spawning processes for more useful error messages on failure
Do not write flaky tests - do not usesetTimeoutin tests; insteadawaitthe condition to be met since you're testing the CONDITION, not TIME PASSING
Verify your test fails withUSE_SYSTEM_BUN=1 bun test <file>and passes withbun bd test <file>- tests are not valid if they pass withUSE_SYSTEM_BUN=1
Avoid shell commands in tests - do not usefindorgrep; use Bun's Glob and built-in tools instead
Test files must end in.test.tsor.test.tsxand be created in the appropriate test folder structure
Files:
test/js/bun/http/bun-server.test.ts
🧠 Learnings (17)
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Applied to files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-11-24T18:35:39.205Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality
Applied to files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.
Applied to files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves
Applied to files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/dev/bundle.test.ts : Organize bundle tests in bundle.test.ts for tests concerning bundling bugs that only occur in DevServer
Applied to files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Applied to files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - tests are not valid if they pass with `USE_SYSTEM_BUN=1`
Applied to files:
test/js/bun/http/bun-server.test.ts
📚 Learning: 2025-10-19T04:55:33.099Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.
Applied to files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr
Applied to files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`
Applied to files:
test/js/bun/http/bun-server.test.ts
📚 Learning: 2025-09-02T05:33:37.517Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22323
File: test/js/web/websocket/websocket-subprotocol.test.ts:74-75
Timestamp: 2025-09-02T05:33:37.517Z
Learning: In Bun's runtime, `await using` with Node.js APIs like `net.createServer()` is properly supported and should not be replaced with explicit cleanup. Bun has extended Node.js APIs with proper async dispose support.
Applied to files:
test/js/bun/http/bun-server.test.tstest/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-11-06T00:58:23.965Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.
Applied to files:
test/js/bun/http/bun-server.test.ts
📚 Learning: 2025-10-25T17:20:19.041Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.
Applied to files:
test/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
test/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: When fixing a Node.js compatibility bug, first verify the expected behavior works in Node.js before testing against Bun's canary version
Applied to files:
test/js/bun/http/bun-websocket-cpu-fixture.js
📚 Learning: 2025-09-02T06:10:17.252Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22323
File: test/js/web/websocket/websocket-subprotocol-strict.test.ts:80-91
Timestamp: 2025-09-02T06:10:17.252Z
Learning: Bun's WebSocket implementation includes a terminate() method, which is available even though it's not part of the standard WebSocket API. This method can be used in Bun test files and applications for immediate WebSocket closure.
Applied to files:
test/js/bun/http/bun-websocket-cpu-fixture.js
🧬 Code graph analysis (3)
packages/bun-usockets/src/loop.c (1)
packages/bun-usockets/src/eventing/epoll_kqueue.c (3)
us_internal_poll_type(91-93)us_poll_change(425-449)us_poll_events(82-84)
test/js/bun/http/bun-server.test.ts (1)
test/harness.ts (1)
bunRun(292-317)
test/js/bun/http/bun-websocket-cpu-fixture.js (1)
test/js/bun/http/bun-server.test.ts (1)
message(631-633)
🪛 Cppcheck (2.18.0)
packages/bun-usockets/src/loop.c
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
[information] Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.
(normalCheckLevelMaxBranches)
[information] Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations. For more details, use --enable=information.
(toomanyconfigs)
🔇 Additional comments (5)
test/js/bun/http/bun-server.test.ts (1)
7-10: LGTM!The test correctly delegates CPU measurement to a fixture and relies on
bunRunthrowing on non-zero exit. The empty stderr assertion provides additional validation. This follows the coding guidelines for spawning Bun processes in tests.packages/bun-usockets/src/eventing/epoll_kqueue.c (2)
231-232: LGTM on the filter equality fix inus_loop_run.Using
==instead of&is correct for kqueue filter comparison sinceEVFILT_READandEVFILT_WRITEare distinct negative values (typically -1 and -2), not bitmasks.
363-367: LGTM on theEV_ONESHOTadditions.Adding
EV_ONESHOTto writable events prevents continuous triggering when no I/O is pending. Combined with the re-arm logic inloop.c, this correctly addresses the kqueue event loop spinning issue.packages/bun-usockets/src/loop.c (2)
378-381: LGTM on updating poll state after one-shot writable fires.This correctly updates the poll type to reflect that kqueue's
EV_ONESHOThas automatically removed the writable registration. The readable bit is preserved if it was active.
392-396: LGTM on re-arming writable for kqueue.When
last_write_failedindicates pending write data and the socket isn't shut down, re-enablingLIBUS_SOCKET_WRITABLEviaus_poll_changecorrectly re-registers the one-shot writable event for kqueue.
What does this PR do?
Fixes a bug where idle WebSocket connections would cause 100% CPU usage on macOS and other BSD systems using kqueue.
Root cause: The kqueue event filter comparison was using bitwise AND (
&) instead of equality (==) when checking the filter type. Combined with missingEV_ONESHOTflags on writable events, this caused the event loop to continuously spin even when no actual I/O was pending.Changes:
Fixed filter comparison in
epoll_kqueue.c: Changedfilter & EVFILT_READtofilter == EVFILT_READ(same forEVFILT_WRITE). The filter field is a value, not a bitmask.Added
EV_ONESHOTflag to writable events: kqueue writable events now use one-shot mode to prevent continuous triggering.Re-arm writable events when needed: After a one-shot writable event fires, the code now properly updates the poll state and re-arms the writable event if another write is still pending.
How did you verify your code works?
Added a test that: