1jehuang / 1jehuang/jcode

cargo test -p jcode-tui --lib deadlocks at the default thread count (ABBA between the render-state lock and the env lock)

Open
#1,141 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

autonomous: no bug priority: medium triage: needs-decision triage: reproducible
Dominant language
Rust
Stars
19.9k
Forks
2.3k
Avg merge
2d 7h
Merged PRs (30d)
30

Description

Summary

cargo test -p jcode-tui --lib never finishes at the default thread count on macOS. It is an ABBA lock-order deadlock between the render-state lock and the env lock, not a slow test. I killed it at 180s; a completed run of the same suite takes ~22s.

Reproduced 5/5 on master (325236119), macOS 26.6.2, Apple silicon (10 cores, so 10 test threads).

Reproduction

git checkout master
cargo test -p jcode-tui --lib      # never returns

--test-threads=1 completes normally, which is why this can look like slowness rather than a hang.

Root cause

Two lock orders exist, and they close a cycle:

  • env → render. ~570 tests call create_test_app(), which calls clear_test_render_state_for_tests()with_render_state_lock. Many of those tests already hold the env lock via with_temp_jcode_home.
  • render → env. A render test that scopes JCODE_HOME holds the render lock and then waits for env.

with_render_state_lock blocks on the render mutex:

// crates/jcode-tui/src/tui/ui.rs
fn with_render_state_lock<T>(body: impl FnOnce() -> T) -> T {
    if render_state_lock_held() {
        return body();
    }

    let _guard = render_state_test_lock();   // blocks
    body()
}

With 10 threads there is enough interleaving for both directions to be in flight at once, and the suite stops making progress.

Evidence

sample on the hung test binary: the main thread is parked forever in test::run_tests_console waiting on CompletedTest, and 11 test threads are all blocked in std::sync::mutex under with_temp_jcode_home. No thread holds the lock and is running, which is the signature of the cycle rather than one slow test.

A/B

Changing exactly one thing — that blocking acquire to a try_lock that proceeds without the lock when it is contended — is sufficient:

run 1 run 2
master HUNG (>180s) HUNG (>180s)
same tree, try_lock only completes, 21.95s completes, 22.59s
full fix branch completes, 22.18s completes, 26.88s

Nothing else was changed. (The runs report failures once they can finish; those are separate pre-existing issues, several already filed. The point here is only hang vs. completes.)

Why try_lock is the right shape

This particular acquire is an incidental reset of render globals before a test builds its app, not a render assertion. Declining it is the behavior that existed before the lock was introduced, and it costs at most some of the pre-existing render-state flakiness. A hung suite costs everything.

Two further details are needed for a complete fix, both of which I hit:

  1. The guard must be reentrant (a depth count, not a bool). Tests nest these through helper wrappers, and a plain bool makes an inner guard's drop clear ownership while the outer guard still holds the mutex.
  2. Two tests take the locks in the wrong order and should be swapped to env-first, matching every other test that needs both: smoothness_benchmark_simulated_streaming_turn_stays_within_budget and test_alt_shift_i_toggles_inline_images_and_persists.

I verified the ordering fixes alone are not sufficient: with both swapped but the acquire still blocking, the suite still deadlocked (killed at 241s). create_test_app is reached from too many call sites to guarantee any order at all.

Notes

A branch carrying the full fix plus regression coverage in tui::ui::lock_order_tests is at https://github.com/ianalitis/jcode/tree/fix/tui-suite-deadlock — it also drops the single-threaded failure count on this machine from 26 to 8. PRs are gated here (pull_request_creation_policy: collaborators_only), so this is a reference branch rather than a PR.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in crates/jcode-tui/src/tui/ui.rs with with_render_state_lock and the render-state helpers, then trace create_test_app and the env-lock helper. Review the two named tests and the regression coverage in tui::ui::lock_order_tests. Run cargo test -p jcode-tui --lib at the default thread count; done means the suite completes without the deadlock and the lock-order tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.