fix(tracing): flush guards on Ctrl-C and doctor-subcommand exit - #6697
Merged
Conversation
std::process::exit runs no destructors, so early_ctrlc's Ctrl-C handler and the Doctor/Gonka::Doctor/Cocoon::Doctor subcommand exits bypassed TracingGuards::drop's take-once-cell flush logic entirely, truncating the log file and/or local Chrome trace on every early exit through those paths. Unlike the SIGTERM race window fixed in #6693, the Ctrl-C gap was a 100%-reproducible bypass on every platform. Add exit_with_flush and flush_and_exit_on_ctrlc in tracing_init.rs, and extract the previously-duplicated lock-hold-across-flush logic into a shared take_and_flush_guard_cells helper reused by TracingGuards::drop, spawn_sigterm_flush_task, and the new Ctrl-C path. tokio::signal::ctrl_c() is cross-platform, so this also closes the non-Unix half of the gap that #6693 left open. Two sites sharing the same pattern (Notify::Test, UrlScheme::Status) and 5 deep-link-feature-gated sites inside handle_url_open share the same bug but are intentionally left out of scope for a follow-up.
bug-ops
enabled auto-merge (squash)
July 28, 2026 15:30
This was referenced Jul 28, 2026
bug-ops
added a commit
that referenced
this pull request
Jul 28, 2026
Notify::Test and UrlScheme::Status --check now call exit_with_flush instead of a bare std::process::exit. handle_url_open (deep-link feature) now returns Option<i32> instead of exiting internally, since tracing_guards is not in scope inside that function; the caller in run() flushes and exits on its behalf. Follow-up from #6697, which fixed the same bug class at other sites but left these 7 untouched.
bug-ops
added a commit
that referenced
this pull request
Jul 28, 2026
Notify::Test and UrlScheme::Status --check now call exit_with_flush instead of a bare std::process::exit. handle_url_open (deep-link feature) now returns Option<i32> instead of exiting internally, since tracing_guards is not in scope inside that function; the caller in run() flushes and exits on its behalf. Follow-up from #6697, which fixed the same bug class at other sites but left these 7 untouched.
bug-ops
added a commit
that referenced
this pull request
Jul 28, 2026
…t sites std::process::exit skipped TracingGuards::drop's flush-on-drop logic at every std::process::exit call site reachable from run() after init_tracing, the same bug class as #6696/#6697/#6698/#6708. Rather than threading exit-signal types through each site, unify on plain anyhow Result propagation: load_config_or_default now returns Result<Config, ConfigLoadError> instead of exiting internally, and every caller (18 total, including the 7 exit sites in commands/bench.rs) uses plain ? so an ordinary Err return unwinds and flushes tracing_guards via normal drop. Closes #6709
bug-ops
added a commit
that referenced
this pull request
Jul 28, 2026
…t sites (#6714) std::process::exit skipped TracingGuards::drop's flush-on-drop logic at every std::process::exit call site reachable from run() after init_tracing, the same bug class as #6696/#6697/#6698/#6708. Rather than threading exit-signal types through each site, unify on plain anyhow Result propagation: load_config_or_default now returns Result<Config, ConfigLoadError> instead of exiting internally, and every caller (18 total, including the 7 exit sites in commands/bench.rs) uses plain ? so an ordinary Err return unwinds and flushes tracing_guards via normal drop. Closes #6709
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
std::process::exitruns no destructors, soearly_ctrlc's Ctrl-C handler and theDoctor/Gonka::Doctor/Cocoon::Doctorsubcommand exits (src/runner.rs) bypassedTracingGuards::drop's take-once-cell flush logic entirely, truncating the log file and/or local Chrome trace on every early exit through those paths. Unlike the SIGTERM race window fixed in log_guard (rolling file WorkerGuard) has same SIGTERM-flush gap as chrome_guard had #6693, the Ctrl-C gap was a 100%-reproducible bypass on every platform.exit_with_flushandflush_and_exit_on_ctrlcinsrc/tracing_init.rs, and extracted the previously-duplicated lock-hold-across-flush logic into a sharedtake_and_flush_guard_cellshelper reused byTracingGuards::drop,spawn_sigterm_flush_task, and the new Ctrl-C path.tokio::signal::ctrl_c()is cross-platform, so this also closes the non-Unix half of the gap that log_guard (rolling file WorkerGuard) has same SIGTERM-flush gap as chrome_guard had #6693 left open.Closes #6696
Scope notes
Two sites sharing the same
std::process::exitpattern (runner.rs:1186Notify::Test,:1231UrlScheme::Status) and 5deep-link-feature-gated sites insidehandle_url_openshare the same bug class but were intentionally left out of scope — a follow-up issue will be filed for them.Review chain
spawn_sigterm_flush_taskclone-and-race pattern).significant: the DRY extraction had silently invertedTracingGuards::drop's flush order (chrome-then-log → log-then-chrome) and left 3 comments stale. Developer confirmed via git archaeology on Local trace files truncated when zeph is killed via pkill (FlushGuard never dropped) #6683/log_guard (rolling file WorkerGuard) has same SIGTERM-flush gap as chrome_guard had #6693 that the original order was incidental (not load-bearing — the two writers are independent files), kept the new order, and corrected the 3 comments. Re-verifiedapprovedon second pass.approved.Test plan
cargo +nightly fmt --checkcargo clippy --profile ci --workspace --all-targets --features "desktop,ide,server,chat,pdf,scheduler,testing" -- -D warningscargo nextest run --config-file .github/nextest.toml --workspace --features "desktop,ide,server,chat,pdf,scheduler" --lib --bins(15258/15258 passed, including alltracing_inittests unmodified)RUSTFLAGS="-D warnings" RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps --workspace --features "desktop,ide,server,chat,pdf,scheduler"gitleaks protect --staged --no-banner --redactexit_with_flush/flush_and_exit_on_ctrlcthemselves — both callstd::process::exitand are untestable in-process (verified noassert_cmd/subprocess-exit test harness precedent exists in the repo).take_and_flush_guard_cellsis indirectly covered via existingTracingGuards::droptests.