Fix: bound socket thread pool#2604
Conversation
…slot wait Documents that the concurrent queue may backlog async blocks while the semaphore limits concurrent handleClient work. Centralizes the 30s slot wait as a named constant. Made-with: Cursor
|
@NewAlexandria is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 857fd9e809
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard self.clientConcurrencyLimit.wait(timeout: .now() + self.clientHandlerSlotWait) == .success else { | ||
| // All handler slots full — reject this connection to apply backpressure. | ||
| let msg = "ERROR: Server busy\n" | ||
| msg.withCString { ptr in _ = write(clientSocket, ptr, strlen(ptr)) } | ||
| close(clientSocket) |
There was a problem hiding this comment.
This timeout check runs only after the async block starts, so under a connection flood the accept loop still enqueues every accepted socket immediately and can accumulate a large backlog of open FDs before any Server busy rejection happens. In the overload case where 64 handlers are blocked, queued closures may not execute promptly, which means the advertised 30s backpressure window is not enforced from accept time and the process can still hit FD/memory pressure before shedding load.
Useful? React with 👍 / 👎.
1 participant
Summary
What changed?
Thread.detachNewThreadfor per-connection socket client handlers with a bounded concurrentDispatchQueue(com.cmuxterm.socket-clients) and aDispatchSemaphore(value: 64)so at most 64 handlers runhandleClientat once.ERROR: Server busy, the socket is closed, and backpressure is applied to callers.Thread.detachNewThread; only per-client dispatch is bounded.asyncwork while the semaphore caps concurrent execution; the 30s wait is a named constant (clientHandlerSlotWait).Why?
Each incoming connection (e.g.
cmux/ Claude Code hooks) used to spawn a newNSThread. When the main thread stalls (e.g. heavy SwiftUI), handler threads that hit@MainActor/DispatchQueue.main.synccan block. With unbounded thread creation, blocked threads pile up. In a real incident that led to 150+ blocked threads and ~10.93 GB memory, leaving the app unrecoverable without a force kill. Bounding concurrency limits worst-case thread/memory growth and surfaces overload explicitly via “Server busy”.Testing
I can follow any suggestions you offer. 🤝
How did you test this change?
cmuxCLI over the socket (e.g.ping,report_*, etc.).ERROR: Server busyand connection close.tests_v2/against a tagged debug build and socket (CMUX_SOCKET=...as per project docs).Test plan
cmuxCLI commands still work (ping, report_*, etc.)stays bounded
tests_v2/) against a tagged debug buildWhat did you verify manually?
reload.sh --tag …, ran CLI ping + a few report commands; optional Instruments/thread count check.”)Demo Video
the app hangs, with a spinning color wheel. Memory is consumed until crash of the machine.
Review Trigger (Copy/Paste as PR comment)
Checklist
docs/is fine;CHANGELOG.mdonly if your repo requires it for this kind of fix)Summary by cubic
Bound socket client handler concurrency to prevent runaway threads and memory spikes during main-thread stalls. Per-connection work now runs on a concurrent queue with a 64-handler cap; saturated connections get "ERROR: Server busy" after 30s.
Thread.detachNewThreadwith a concurrentDispatchQueue(com.cmuxterm.socket-clients) gated by aDispatchSemaphore(64).ERROR: Server busyand close the socket to apply backpressure.Written for commit 857fd9e. Summary will update on new commits.