Skip to content

acp: Poll the client connection future on a dedicated thread - #62259

Merged
mikayla-maki merged 2 commits into
mainfrom
fix-acp-dev-stack-overflow-28dd44909cb1496c
Aug 15, 2026
Merged

acp: Poll the client connection future on a dedicated thread#62259
mikayla-maki merged 2 commits into
mainfrom
fix-acp-dev-stack-overflow-28dd44909cb1496c

Conversation

@yeskunall

Copy link
Copy Markdown
Member

Some context behind this change: I’ve been meaning to look into this because I kept running into this crash when working on other fixes. Everything worked fine in dev builds, right up until you tried to create a new Agent Thread using an ACP adapter. The crash reports in this instance were throwing me off: I was seeing different things each time.

So, I had Fable look into this. It did so by binary-searching the minimum thread stack on which a probe replicating Zed’s exact handler chain can dispatch one message. Full methodology, probe source, and raw numbers can be found in this gist. The results of the probe are as follows:

SDK Minimum stack (dev profile) vs. 512 KiB GCD budget
1.3.0 409,600–413,696 B fit, ~100 KiB headroom
2.0.0 507,904–512,000 B entire budget before runtime overhead

The oversized frames are monomorphized into agent_servers, not the SDK crate -- a [profile.dev.package] opt-level override on agent-client-protocol does not fix this (see gist), and optimized builds collapse the frames entirely, which is why only dev builds crashed. It found that the real signature is fault_address == stack_pointer on a com.apple.root.default-qos thread inside the ACP dispatch specialization, which I then had it verify across six local .ips reports. It seems in #61570, we pushed the dispatch chain past the GCD budget, explained further below:

AcpConnection::stdio polled the ACP client connection future via background_spawn, which on macOS executes runnables on GCD’s global-queue workers. Those threads have kernel-fixed, unconfigurable 512 KiB stacks. In unoptimized builds, the SDK’s chained-handler dispatch needs ~0.5 MiB of stack per inbound message (again, see linked gist), so the first message overflows the guard page and takes the process down.

Therefore, this 512 KiB constraint is macOS-only. Linux doesn’t use GCD -- GPUI spawns its own std::thread workers (2 MiB Rust default). Windows uses the OS thread pool, which inherits the executable’s stack reserve -- 1 MB linker default, but Zed already bumps it to 8 MiB in crates/zed/build.rs:88 (see TODO comment).


Release Notes:

  • N/A

In unoptimized builds, the ACP client's monomorphized dispatch chain
needs ~0.5 MiB of stack per inbound message. `background_spawn` polls
futures on macOS GCD workers whose stacks are kernel-fixed at 512 KiB,
so dev builds crashed with a stack overflow as soon as an external
agent sent its first message. Poll the connection future on a dedicated
thread with the standard 2 MiB stack instead, exposing `spawn_dedicated`
on gpui's `BackgroundExecutor` to do so.
@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Aug 6, 2026
@zed-community-bot zed-community-bot Bot added the staff Pull requests authored by a current member of Zed staff label Aug 6, 2026
@zed-industries-bot

This comment was marked as resolved.

mikayla-maki
mikayla-maki previously approved these changes Aug 14, 2026

@mikayla-maki mikayla-maki left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@mikayla-maki
mikayla-maki added this pull request to the merge queue Aug 15, 2026
Merged via the queue into main with commit 9bde578 Aug 15, 2026
36 checks passed
@mikayla-maki
mikayla-maki deleted the fix-acp-dev-stack-overflow-28dd44909cb1496c branch August 15, 2026 00:33
playdohface pushed a commit to playdohface/zed that referenced this pull request Aug 29, 2026
…ustries#62259)

Some context behind this change: I’ve been meaning to look into this
because I kept running into this crash when working on
[other](zed-industries#61040) fixes.
Everything worked fine in dev builds, right up until you tried to create
a new Agent Thread using an ACP adapter. The crash reports in this
instance were throwing me off: I was seeing different things each time.

So, I had Fable look into this. It did so by binary-searching the
minimum thread stack on which a probe replicating Zed’s exact handler
chain can dispatch one message. Full methodology, probe source, and raw
numbers can be found in [this
gist](https://gist.github.com/yeskunall/2ac2b00f51389d9388d607974f6f8a04).
The results of the probe are as follows:

| SDK | Minimum stack (dev profile) | vs. 512 KiB GCD budget |
|---|---|---|
| 1.3.0 | 409,600–413,696 B | fit, ~100 KiB headroom |
| 2.0.0 | 507,904–512,000 B | entire budget before runtime overhead |

The oversized frames are monomorphized into `agent_servers`, not the SDK
crate -- a `[profile.dev.package]` opt-level override on
`agent-client-protocol` does **not** fix this (see gist), and optimized
builds collapse the frames entirely, which is why only dev builds
crashed. It found that the real signature is `fault_address ==
stack_pointer` on a `com.apple.root.default-qos` thread inside the ACP
dispatch specialization, which I then had it verify across six local
`.ips` reports. It seems in zed-industries#61570, we pushed the dispatch chain past
the GCD budget, explained further below:

`AcpConnection::stdio` polled the ACP client connection future via
`background_spawn`, which on macOS executes runnables on
[GCD’s](https://developer.apple.com/documentation/DISPATCH) global-queue
workers. Those threads have kernel-fixed, unconfigurable [512 KiB
stacks](https://github.com/apple-oss-distributions/libpthread/blob/42d026df5b07825070f60134b980a1ec2552dfee/kern/kern_internal.h#L154).
In unoptimized builds, the SDK’s chained-handler dispatch needs **~0.5
MiB of stack per inbound message** (again, see linked gist), so the
first message overflows the guard page and takes the process down.

Therefore, this 512 KiB constraint is **macOS-only**. Linux doesn’t use
GCD -- GPUI [spawns its own `std::thread`
workers](https://github.com/zed-industries/zed/blob/82878540b5410b288a2c92cb9ee5675533e4d807/crates/gpui_linux/src/linux/dispatcher.rs#L39)
([2 MiB Rust
default](https://github.com/rust-lang/rust/blob/59807616e1fa2540724bfbac14d7976d7e4a3860/library/std/src/sys/thread/unix.rs#L26)).
Windows uses [the OS thread
pool](https://github.com/zed-industries/zed/blob/82878540b5410b288a2c92cb9ee5675533e4d807/crates/gpui_windows/src/dispatcher.rs#L68),
which [inherits the executable’s stack
reserve](https://github.com/MicrosoftDocs/sdk-api/blob/4502fff176b3b56beddb6a63c9f980377b11ba9b/sdk-api-src/content/threadpoolapiset/nf-threadpoolapiset-setthreadpoolstackinformation.md?plain=1#L58)
-- [1 MB linker
default](https://github.com/MicrosoftDocs/win32/blob/2eb6588c6703d31599285bbb06563c3d41b57590/desktop-src/ProcThread/thread-stack-size.md?plain=1#L17),
but Zed already bumps it to 8 MiB in
[crates/zed/build.rs:88](https://github.com/zed-industries/zed/blob/82878540b5410b288a2c92cb9ee5675533e4d807/crates/zed/build.rs#L88)
(see [TODO
comment](https://github.com/zed-industries/zed/blob/82878540b5410b288a2c92cb9ee5675533e4d807/crates/zed/build.rs#L87)).

---

Release Notes:

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

Labels

cla-signed The user has signed the Contributor License Agreement staff Pull requests authored by a current member of Zed staff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants