Skip to content

fix(tauri): prevent window-state capture deadlocks - #677

Merged
shantur merged 2 commits into
NeuralNomadsAI:devfrom
pascalandr:fix/issue-676-tauri-window-flush
Sep 6, 2026
Merged

fix(tauri): prevent window-state capture deadlocks#677
shantur merged 2 commits into
NeuralNomadsAI:devfrom
pascalandr:fix/issue-676-tauri-window-flush

Conversation

@pascalandr

@pascalandr pascalandr commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #676.

This PR now addresses both mechanisms behind the window-state hang, rather than only reducing the thread count:

  • one lazy named worker, a capacity-one wakeup queue, and a 250 ms debounce;
  • native getters run before any client-state lock, including explicit close/shutdown captures;
  • move/resize/scale-factor/zoom handlers enqueue only the latest geometry per known window and never acquire the disk writer lock;
  • serialized publication merges queued captures, preserves normal bounds across maximize/fullscreen, and seeds clamped restore bounds before maximizing;
  • rollback preserves events arriving during snapshot/clear/disable/removal publication, including the window whose policy is tentatively changing; a successful destructive commit discards those speculative events;
  • shutdown closes capture admission, joins the worker without retaining the scheduler/writer mutex, drains an omitted wakeup, and retries captures merged by a failed flush before ownership release;
  • no application handle retained by the idle worker;
  • ownership-file validation in load_window moved outside the shared in-memory state lock.

Electron already debounces native geometry; its production code is unchanged, with a new parity regression covering bursts, maximization, explicit flush, and closed-window cleanup.

Why the initial version was insufficient

The original head bb38b1a2 fixed thread proliferation, but capture_window_in_memory still held write_lock across native getters. A slow fsync could block the UI through that mutex; a secondary-thread capture could also hold it while waiting for the same UI thread that was handling a move/resize event.

This follow-up retains the useful scheduler behavior and completes the capture/persistence separation in the same PR. No competing #676 PR is being introduced.

Before/after evidence

An external Rust harness compiles the exact production capture, flush, release and scheduler function bodies, replacing only the native/disk adapters. It counts real Windows OS threads and uses controlled channel gates rather than relying on a randomly timed freeze.

Assertion Base 52f0e629 Initial PR bb38b1a2 Completed PR
128 flush requests +128 threads +1 thread +1 thread
Capture completes before blocked disk publication is released FAIL FAIL PASS
UI can complete move/resize while a secondary capture waits for a native getter FAIL FAIL PASS
Explicit capture/flush retains geometry PASS PASS PASS
Future envelopes and ephemeral windows remain protected PASS PASS PASS

Ten repetitions per compared version produced the same outcomes. This is a controlled reproduction of the concurrency mechanisms, not a claim of having run the reporter's macOS GUI session.

Committed regression coverage

The crate now includes 19 focused scheduler/capture tests (the original three plus sixteen additional regressions), using the actual ClientState, native-read seam, scheduler and atomic-write adapter. Coverage includes:

  • 1,000 captures during a stalled disk write; bounded memory and latest geometry;
  • no client-state mutex held by native getters; simulated secondary getter/UI event cycle;
  • one worker/one trailing wakeup, callback lifetime, concurrent stop, cheap late submission and no restart after release;
  • independent windows, maximization/fullscreen, zoom, unknown-window rejection;
  • successful and failed destructive publication, simultaneous events on the mutating and another window, retry after failed flush;
  • final persistence and cross-host ownership handoff;
  • secondary ownership and future-envelope fencing.

Validation

  • Full Windows Tauri crate: 158 tests passed, including default test parallelism.
  • Electron native suite: 190 tests passed.
  • UI/Electron typechecks passed.
  • Official Tauri resource preparation and server/UI builds passed during verification.
  • Windows release executable builds successfully.
  • cargo fmt --check, git diff --check, and workflow YAML parsing passed.
  • Added an independent macOS ARM64 (macos-26) Tauri test job to exercise the crate on the reported platform; packaging now depends on it as well. Its remote result must be checked before merging.

CI dependency and limits

The inherited Linux automation-registry test failure remains isolated in #671. #675 was closed as its duplicate and is not part of this fix. No duplicate registry patch or unrelated PR was merged into this branch. After #671 is integrated, update this branch and require the complete matrix before merge.

Windows controlled reproductions and tests are not a substitute for a macOS ARM64 interactive launch/move/resize smoke test. Persistent disk failures are still logged and cannot guarantee saving to an unavailable disk. No claim of absolute absence of side effects is made.

Maintenance note: packages/tauri-app/src-tauri/src/client_state.rs remains oversized (about 1,160 lines); focused capture and scheduler code/tests are kept in sibling modules instead of undertaking an unrelated refactor.

Replace the per-move and per-resize thread spawn with one lazily started, bounded debounce worker. Window bounds are still captured immediately, while disk persistence is coalesced after 250 ms so native event handling cannot accumulate hundreds of flush threads behind the client-state write lock.

Keep at most one trailing request while a write is active and drain pending work before releasing cross-host ownership. Pass AppHandle values through the queue instead of retaining one while idle so the worker does not extend the application lifetime.

Add regression coverage for event bursts, requests arriving during an active flush, and prompt draining during shutdown. Fixes NeuralNomadsAI#676.

@pascalandr pascalandr left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Gatekeeper review of bb38b1a2

No blocking code findings remain.

The scheduler keeps one worker and a one-item wakeup queue, resets the debounce while events arrive, preserves one trailing flush when an event arrives during persistence, drains queued state before releasing cross-process ownership, and permanently rejects post-release scheduling. The shutdown lock order avoids joining a worker while holding the write lock it needs.

Validation

  • Complete Tauri suite: 142/142 passed.
  • Focused scheduler regressions cover burst coalescing, trailing flushes, and disconnect draining.
  • Rust formatting and git diff --check passed.

Gate status

Code-approved, CI-blocked. The first Linux job failed on the inherited automation-registry test fixed by #671; the later rerun was externally cancelled during checkout. Rerun the complete PR matrix after #671 lands before merging.

Complete NeuralNomadsAI#676's worker-only fix by reading native geometry before any client-state mutex and queueing only the latest capture per known window. Move/resize/DPI and zoom handlers no longer wait on atomic publication, and restore seeds clamped normal bounds before maximization. Ownership validation also leaves the in-memory mutex free during disk reads.

Merge captures under the serialized writer before rollback snapshots. Preserve admission during tentative clear, disable and removal so failed publication cannot lose concurrent events; discard those speculative events after a successful destructive commit. Close admission before draining, retry previously merged but unpublished captures, and retain cross-host fencing until final persistence completes.

Keep a single lazy worker and capacity-one queue, but consume callback ownership while idle and release the scheduler mutex before joining. Concurrent stop callers wait for the same drain while late UI submissions return promptly.

Add sixteen Rust regressions beyond the original three scheduler tests, an Electron parity test, and an independent macOS ARM64 CI test gate. Windows validation passes all 158 Rust tests at default parallelism, all 190 Electron tests, UI/Electron typechecks and a release build. The same source-extraction harness that reproduces the old disk/UI lock failures now passes all five assertions over ten repetitions. Linux CI still depends on the separate NeuralNomadsAI#671 registry fix; an interactive macOS smoke test remains necessary.
@pascalandr pascalandr changed the title fix(tauri): coalesce window state flushes fix(tauri): prevent window-state capture deadlocks Sep 6, 2026
@pascalandr

Copy link
Copy Markdown
Contributor Author

Follow-up published as 727197d (fast-forward, retaining the original PR). The earlier worker-only review applies to bb38b1a, not to the complete capture fix. The same five-assertion source-extraction harness now passes 10/10 repetitions; the old commit still reproduces both lock failures. All 158 Windows Rust tests pass at default parallelism, all 190 Electron tests pass, and the 19 committed scheduler/capture regressions additionally pass 20/20 repeated runs. Release build, typechecks and formatting checks pass. The PR description documents the implementation and limits; the new independent macOS ARM64 test job must be checked before merge. The existing Linux gate remains dependent on #671, and #675 stays closed as its duplicate. No competing #676 PR or unrelated registry patch was created.

@pascalandr

Copy link
Copy Markdown
Contributor Author

CI results are now confirmed for 727197d, run https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/34035208752 : macOS 26 ARM64 passed all 149 platform-applicable Rust tests, including all 19 scheduler/capture regressions; Windows passed all 158 Rust tests. Linux reached the server suite and failed only the inherited 'prunes stale registry pressure before limiting discovery' test (375 passed, 1 failed, 2 skipped), with the same ENOENT on /tmp/codenomad-automation-stale-*/CodeNomad/automation-bridges fixed by #671. Packaging was consequently skipped. The separate failed artifact-comment job inspected a superseded cancelled run, not a native test failure. No additional production change is indicated by these logs. The remaining merge gate is #671 integration followed by the full PR matrix; an interactive macOS GUI smoke test is still recommended.

@shantur
shantur merged commit b306b44 into NeuralNomadsAI:dev Sep 6, 2026
7 of 14 checks passed
pascalandr added a commit that referenced this pull request Sep 6, 2026
Update #672 with dev at 81aa4d2, including the integrated #671 registry isolation, #674 POSIX shell selection and #677 window persistence fixes. This brings in the previously missing Linux CI prerequisite without duplicating its patch.

Resolve the sole conflict in pr-build.yml by retaining both the OpenCode compatibility matrix and macOS ARM64 Tauri test job, and requiring both before packaging. Every non-build job is structurally identical to its originating parent and every parent build dependency is preserved. No application code needed manual conflict resolution.

Validation after integration: 391 server tests passed with 2 skipped, all 192 Electron tests passed, and server/UI/Electron typechecks passed. YAML parsing, exact parent-job comparisons and git diff --check pass. The complete remote matrix must rerun on the combined branch before merging the PR.
pascalandr added a commit that referenced this pull request Sep 6, 2026
## Summary

Closes #669.

CodeNomad V2 requires the official OpenCode V2 shared-service lifecycle.
A legacy binary must not be accepted merely because `--version`
succeeds, nor should opening a folder display a raw legacy CLI help
dump. This is an actionable compatibility fix, **not V1 runtime support
or automatic installation**: install/select `opencode2` to open the
workspace.

- Validate the selected executable using only `--version` and `service
--help`; neither probe starts or changes the daemon.
- Require positive service-help evidence for `start`, `status`, and
`get`, without pinning a release number or rejecting custom version
labels.
- Recognize legacy root help, including ANSI/CRLF, stdout/stderr, and
exit-zero wrappers. Preserve the localized V2 diagnosis for previously
saved incompatible paths during the actual host/WSL lifecycle.
- Run both validation probes asynchronously with a 5-second subprocess
timeout and 64 KiB output limit, leaving the shared backend responsive
while a CLI is slow.
- Treat saved versions as display metadata only: explicit add/browse
always validates the path again.
- Match the compatibility error code precisely, including its JSON HTTP
envelope, so unrelated missing-file/configuration errors are not hidden
by a matching filename.
- Reuse the existing translated message in all ten locales. No daemon
ownership, V1 fallback, package update, or native-shell-selection
change.

## Follow-up verification of the original PR

The original head `226e2d7b` already fixed the exact reported failure
with a real V1 binary. Independent rechecking found and corrected
adjacent gaps in this same PR:

| Check | Original PR | Completed PR |
| --- | --- | --- |
| Real OpenCode 1.18.25 rejected; real V2 accepted | PASS | PASS |
| Empty/unrelated successful help rejected | FAIL | PASS |
| Legacy help with exit zero localized at launch | FAIL | PASS |
| Other backend work proceeds during slow probes | FAIL | PASS |
| Explicit add rechecks a persisted V1 version | FAIL | PASS |
| Unrelated diagnostic mentioning `opencode_v2_required` retained | FAIL
| PASS |

The new regression tests were run against the old production code first:
three assertions failed (false capability acceptance, exit-zero help,
unrelated-error masking), then passed after the fix. A separate
source-extraction check executes the unchanged published probe and
component validation body: two 250 ms subprocess delays block the old
probe for about 638 ms, versus a Promise returned in about 5 ms after
the fix; a cached V1 version previously produced zero validation calls
and now produces one and rejects the path.

## Validation of the compatibility fix (`903d9159`)

- Full server suite on Windows: **391 passed, 2 skipped**.
- Electron native suite: **189 passed**.
- Focused compatibility/lifecycle/updater/HTTP/UI diagnostic suite: **79
passed** on Windows.
- Server, UI and Electron typechecks passed; server and UI production
builds passed.
- `git diff --check` and workflow YAML validation passed.
- Real CLI checks: V1 **1.18.25** refused with `opencode_v2_required`;
V2 beta builds **19151/19192** accepted; authenticated discovery of the
existing shared daemon succeeds. No service stop/restart or settings
mutation was performed.
- Actual subprocess fixtures exercise spaced/apostrophe/Unicode paths,
stdout/stderr and exit codes. HTTP integration proves that validation
neither mutates preferences nor blocks another request.
- Existing regressions retain stopped/running daemon discovery, startup
environment isolation, password-error redaction, deadlines, WSL command
forwarding, authenticated health, shared-service ownership and updater
behavior.
- Independent focused CI matrix passed on **Linux and macOS 26 ARM64**
(77 passed, 2 Windows-only skips each) and **Windows** (79 passed), in
run `34036886140`.

## Update with current `dev` / conflict resolution

Updated this branch with `dev` at `81aa4d24`, which now includes **#671,
#674 and #677**. The only textual conflict was in
`.github/workflows/pr-build.yml`, where #672 and #677 added CI jobs at
the same location. Both the OpenCode compatibility matrix and the macOS
ARM64 Tauri tests are retained, with both required by the package-build
gate. Parsed workflow comparisons verify that every non-build job is
identical to its originating parent and no build dependency was dropped.

The previously blocking Linux automation-registry fix **#671 is now
included through `dev`**, rather than duplicated. The full CI matrix is
rerunning on this combined state; require that result before merge.

Local checks after resolving the conflict: **391 server tests passed, 2
skipped; 192 Electron tests passed; server/UI/Electron typechecks
passed; workflow parent-preservation assertions and `git diff --check`
passed**. The conflict resolution itself changes no application code.

Automated CLI/HTTP tests are not an interactive Electron/macOS ARM64
smoke test on the reporter's machine, and cannot establish the absolute
absence of every possible side effect.

Maintenance: files already touched by the original PR remain oversized:
`packages/server/src/api-types.ts` (536 lines) and
`packages/ui/src/App.tsx` (855 lines). This follow-up does not grow
those files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tauri app hangs on launch — main thread deadlocked behind hundreds of schedule_flush threads contending on window-state lock

2 participants