Skip to content

gpui: Fix deadlock in performance profiler and reenable it - #61584

Merged
Anthony-Eid merged 1 commit into
mainfrom
fix-gpui-profiler-deadlock
Jul 24, 2026
Merged

gpui: Fix deadlock in performance profiler and reenable it#61584
Anthony-Eid merged 1 commit into
mainfrom
fix-gpui-profiler-deadlock

Conversation

@Anthony-Eid

Copy link
Copy Markdown
Contributor

Summary

PR #58942 disabled the performance profiler within Zed because there was a race condition that caused Zed to hang forever due to a deadlock involving the foreground thread. This PR fixes the deadlock and re-enables the performance profiler.

The deadlock happened because ThreadTimings::drop() locks GLOBAL_THREAD_TIMINGS, and that drop could run in places where GLOBAL_THREAD_TIMINGS was already locked: the collection paths (get_all_timings, take_all_stats, set_trace_enabled(false)) held the global lock while upgrading and then dropping per-thread Arc<GuardedTaskTimings> handles. If a worker thread exited in that window (e.g. GCD reclaiming an idle thread), the collector inherited the last strong reference, and dropping it ran ThreadTimings::drop -> GLOBAL_THREAD_TIMINGS.lock() reentrantly on the same thread. The spinlock is not reentrant, so the thread spun forever while holding the lock, hanging every other thread that touched the profiler.

The fix: hold GLOBAL_THREAD_TIMINGS only long enough to upgrade the Weak handles (upgraded_thread_timings()), and release it before any per-thread buffer is locked, copied, or dropped. A last-reference drop now always runs with the global lock free. As a side benefit, the up-to-16MiB per-thread buffer copies no longer happen under the global lock.

Diagram

sequenceDiagram
    participant C as Collector thread (get_all_timings)
    participant G as GLOBAL_THREAD_TIMINGS (spin::Mutex)
    participant T as Worker thread (exiting)

    Note over T: holds the only strong Arc<br/>in its THREAD_TIMINGS TLS
    C->>G: lock() — guard held for entire collection
    C->>C: Weak::upgrade() (strong: 1 → 2)
    T->>T: thread exits, TLS destructor drops its Arc (strong: 2 → 1)
    C->>C: temp Arc dropped at end of iteration (strong: 1 → 0)
    C->>C: ThreadTimings::drop() runs on collector thread
    C->>G: lock() again — already held by this thread
    Note over C,G: spin lock is not reentrant → spins forever,<br/>every other profiler user spins behind it

    Note over C,T: Fix: upgrade all Weaks under the lock, release it,<br/>then lock/copy/drop per-thread handles — the reentrant<br/>drop can now only ever run with the global lock free
Loading

Release Notes:

  • Fixed a deadlock in the performance profiler and re-enabled it (zed: open performance profiler)

@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Jul 24, 2026
@zed-community-bot zed-community-bot Bot added the staff Pull requests authored by a current member of Zed staff label Jul 24, 2026
@Anthony-Eid
Anthony-Eid added this pull request to the merge queue Jul 24, 2026
Merged via the queue into main with commit c16d19c Jul 24, 2026
50 checks passed
@Anthony-Eid
Anthony-Eid deleted the fix-gpui-profiler-deadlock branch July 24, 2026 15:37
mdz-axo added a commit to mdz-axo/zed-kask that referenced this pull request Jul 25, 2026
Upstream changes (zed-industries/zed main, 27 commits):
- agent: Add agent.compaction_model setting for context compaction (zed-industries#60012)
- agent: Show effort selector for anthropic compatible providers (zed-industries#61579)
- acp: Update agent-client-protocol SDK to 2.0.0 (zed-industries#61570)
- client: Extract proxy handshakes into new proxy_handshake crate (zed-industries#61427)
- collab: Fix multiworkspace location out of sync bugs (zed-industries#61598)
- editor: Fix sticky header drag cancels autoscroll (zed-industries#53592)
- editor: Fix crash when copying and pasting using multiple cursors (zed-industries#61545)
- editor: Skip untitled buffers when saving a multi-buffer (zed-industries#61380)
- gpui: Fix images not being drawn with rounded corners with ObjectFit::Cover (zed-industries#61383)
- gpui: Fix deadlock in performance profiler and reenable it (zed-industries#61584)
- git_ui: Prevent Git panel bindings in repository selector (zed-industries#61282)
- language_model: Add explicit OpenAI conversation compaction and fix Anthropic compaction (zed-industries#61370)
- markdown: Fix squashed Mermaid diagrams in markdown preview (zed-industries#61260)
- Opus 5 BYOK Support (zed-industries#61596)
- repl: Show add-cell controls in empty notebooks (zed-industries#61329)
- search: Escape seeded buffer search query in regex mode (zed-industries#57748)
- settings: Fix VS Code import appending duplicate file associations (zed-industries#61355)
- settings: Split VSCode and Zed keymap files (zed-industries#61532)
- Treat blank spawn_agent session IDs as absent (zed-industries#60893)
- worktree: Reload git state when a watcher rescan covers a repository (zed-industries#61541)
- Plus 7 more minor fixes.

Merge fixes:
- crates/agent/src/thread.rs: replay_tool_call used 'message_ix' (undefined)
  after auto-merge; renamed to 'owning_message_ix' (the parameter name).
- Cargo.toml: Removed stale workspace members hkask-wallet and hkask-git-cas
  (both directories deleted in prior commits but workspace entries remained).
- kask/crates/hkask-regulation/src/wallet_manager.rs: Stubbed consume() and
  settle_rjoules() on WalletBudgetPort — these were API-key encumbrance
  operations from the deleted hkask-wallet crate; regulation tracks per-agent
  gas balances, not per-key encumbrances.
- kask/crates/hkask-regulation/src/wallet_gas_calibrator.rs: Fixed test to
  use crate::agent_wallet_store::WalletStore instead of hkask_storage::WalletStore.
- kask/crates/hkask-regulation/Cargo.toml: Added tokio macros feature to
  dev-dependencies for #[tokio::test].
- kask/crates/kask_bridge/Cargo.toml: Added futures dependency (needed by
  context_injector.rs for futures::executor::block_on).
- kask/crates/kask_bridge/src/context_injector.rs: Fixed futures_util::executor
  to futures::executor (futures-util doesn't include executor module).

Release Notes:

- N/A
0arm pushed a commit to 0arm/zed that referenced this pull request Jul 26, 2026
…tries#61584)

### Summary

PR zed-industries#58942 disabled the performance profiler within Zed because there was
a race condition that caused Zed to hang forever due to a deadlock
involving the foreground thread. This PR fixes the deadlock and
re-enables the performance profiler.

The deadlock happened because `ThreadTimings::drop()` locks
`GLOBAL_THREAD_TIMINGS`, and that drop could run in places where
`GLOBAL_THREAD_TIMINGS` was already locked: the collection paths
(`get_all_timings`, `take_all_stats`, `set_trace_enabled(false)`) held
the global lock while upgrading and then dropping per-thread
`Arc<GuardedTaskTimings>` handles. If a worker thread exited in that
window (e.g. GCD reclaiming an idle thread), the collector inherited the
last strong reference, and dropping it ran `ThreadTimings::drop` ->
`GLOBAL_THREAD_TIMINGS.lock()` reentrantly on the same thread. The
spinlock is not reentrant, so the thread spun forever while holding the
lock, hanging every other thread that touched the profiler.

The fix: hold `GLOBAL_THREAD_TIMINGS` only long enough to upgrade the
`Weak` handles (`upgraded_thread_timings()`), and release it before any
per-thread buffer is locked, copied, or dropped. A last-reference drop
now always runs with the global lock free. As a side benefit, the
up-to-16MiB per-thread buffer copies no longer happen under the global
lock.

### Diagram

```mermaid
sequenceDiagram
    participant C as Collector thread (get_all_timings)
    participant G as GLOBAL_THREAD_TIMINGS (spin::Mutex)
    participant T as Worker thread (exiting)

    Note over T: holds the only strong Arc<br/>in its THREAD_TIMINGS TLS
    C->>G: lock() — guard held for entire collection
    C->>C: Weak::upgrade() (strong: 1 → 2)
    T->>T: thread exits, TLS destructor drops its Arc (strong: 2 → 1)
    C->>C: temp Arc dropped at end of iteration (strong: 1 → 0)
    C->>C: ThreadTimings::drop() runs on collector thread
    C->>G: lock() again — already held by this thread
    Note over C,G: spin lock is not reentrant → spins forever,<br/>every other profiler user spins behind it

    Note over C,T: Fix: upgrade all Weaks under the lock, release it,<br/>then lock/copy/drop per-thread handles — the reentrant<br/>drop can now only ever run with the global lock free
```

Release Notes:

- Fixed a deadlock in the performance profiler and re-enabled it (`zed:
open performance profiler`)
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
…tries#61584)

### Summary

PR zed-industries#58942 disabled the performance profiler within Zed because there was
a race condition that caused Zed to hang forever due to a deadlock
involving the foreground thread. This PR fixes the deadlock and
re-enables the performance profiler.

The deadlock happened because `ThreadTimings::drop()` locks
`GLOBAL_THREAD_TIMINGS`, and that drop could run in places where
`GLOBAL_THREAD_TIMINGS` was already locked: the collection paths
(`get_all_timings`, `take_all_stats`, `set_trace_enabled(false)`) held
the global lock while upgrading and then dropping per-thread
`Arc<GuardedTaskTimings>` handles. If a worker thread exited in that
window (e.g. GCD reclaiming an idle thread), the collector inherited the
last strong reference, and dropping it ran `ThreadTimings::drop` ->
`GLOBAL_THREAD_TIMINGS.lock()` reentrantly on the same thread. The
spinlock is not reentrant, so the thread spun forever while holding the
lock, hanging every other thread that touched the profiler.

The fix: hold `GLOBAL_THREAD_TIMINGS` only long enough to upgrade the
`Weak` handles (`upgraded_thread_timings()`), and release it before any
per-thread buffer is locked, copied, or dropped. A last-reference drop
now always runs with the global lock free. As a side benefit, the
up-to-16MiB per-thread buffer copies no longer happen under the global
lock.

### Diagram

```mermaid
sequenceDiagram
    participant C as Collector thread (get_all_timings)
    participant G as GLOBAL_THREAD_TIMINGS (spin::Mutex)
    participant T as Worker thread (exiting)

    Note over T: holds the only strong Arc<br/>in its THREAD_TIMINGS TLS
    C->>G: lock() — guard held for entire collection
    C->>C: Weak::upgrade() (strong: 1 → 2)
    T->>T: thread exits, TLS destructor drops its Arc (strong: 2 → 1)
    C->>C: temp Arc dropped at end of iteration (strong: 1 → 0)
    C->>C: ThreadTimings::drop() runs on collector thread
    C->>G: lock() again — already held by this thread
    Note over C,G: spin lock is not reentrant → spins forever,<br/>every other profiler user spins behind it

    Note over C,T: Fix: upgrade all Weaks under the lock, release it,<br/>then lock/copy/drop per-thread handles — the reentrant<br/>drop can now only ever run with the global lock free
```

Release Notes:

- Fixed a deadlock in the performance profiler and re-enabled it (`zed:
open performance profiler`)
playdohface pushed a commit to playdohface/zed that referenced this pull request Aug 29, 2026
…tries#61584)

### Summary

PR zed-industries#58942 disabled the performance profiler within Zed because there was
a race condition that caused Zed to hang forever due to a deadlock
involving the foreground thread. This PR fixes the deadlock and
re-enables the performance profiler.

The deadlock happened because `ThreadTimings::drop()` locks
`GLOBAL_THREAD_TIMINGS`, and that drop could run in places where
`GLOBAL_THREAD_TIMINGS` was already locked: the collection paths
(`get_all_timings`, `take_all_stats`, `set_trace_enabled(false)`) held
the global lock while upgrading and then dropping per-thread
`Arc<GuardedTaskTimings>` handles. If a worker thread exited in that
window (e.g. GCD reclaiming an idle thread), the collector inherited the
last strong reference, and dropping it ran `ThreadTimings::drop` ->
`GLOBAL_THREAD_TIMINGS.lock()` reentrantly on the same thread. The
spinlock is not reentrant, so the thread spun forever while holding the
lock, hanging every other thread that touched the profiler.

The fix: hold `GLOBAL_THREAD_TIMINGS` only long enough to upgrade the
`Weak` handles (`upgraded_thread_timings()`), and release it before any
per-thread buffer is locked, copied, or dropped. A last-reference drop
now always runs with the global lock free. As a side benefit, the
up-to-16MiB per-thread buffer copies no longer happen under the global
lock.

### Diagram

```mermaid
sequenceDiagram
    participant C as Collector thread (get_all_timings)
    participant G as GLOBAL_THREAD_TIMINGS (spin::Mutex)
    participant T as Worker thread (exiting)

    Note over T: holds the only strong Arc<br/>in its THREAD_TIMINGS TLS
    C->>G: lock() — guard held for entire collection
    C->>C: Weak::upgrade() (strong: 1 → 2)
    T->>T: thread exits, TLS destructor drops its Arc (strong: 2 → 1)
    C->>C: temp Arc dropped at end of iteration (strong: 1 → 0)
    C->>C: ThreadTimings::drop() runs on collector thread
    C->>G: lock() again — already held by this thread
    Note over C,G: spin lock is not reentrant → spins forever,<br/>every other profiler user spins behind it

    Note over C,T: Fix: upgrade all Weaks under the lock, release it,<br/>then lock/copy/drop per-thread handles — the reentrant<br/>drop can now only ever run with the global lock free
```

Release Notes:

- Fixed a deadlock in the performance profiler and re-enabled it (`zed:
open performance profiler`)
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.

2 participants