feat: best-effort flush of in-flight events at crash time (#13)

[krassx]

Problem

A producer only queues an entry into the capture worker's channel; the worker appends it to the part file later. Events still queued (or mid-append) when the process dies are lost, and those are the last breadcrumbs, logs and network events before the fault. Measured with a subprocess crash test (1000 events, then SIGSEGV): without this change only 16-44 of 1000 events were recovered (15/250 for a smaller burst).

Design

bugsee_core::crash_flush:

• One process-wide wake pipe (Windows: event pair) and one helper thread, created at launch (never in the handler) and re-created in a fork() child. The wake handles are never closed, so there is no descriptor-reuse race and no SIGPIPE for C hosts.
• After the marker and frames are on disk, the fatal handler (signal handler / Windows exception filter) raises a dying flag, takes an atomic ticket, does one non-blocking write(2) (SetEvent) and polls an atomic ack for at most 50 ms (poll / ack event + QPC).
• The helper posts the existing Flush message through the normal FIFO channel to the capture worker; when the worker acks, every entry queued before the crash has been appended, and the helper publishes the ticket.
• Signal safety: the crashing thread only uses atomics, getpid, pthread_self, one non-blocking write, clock_gettime and poll. No allocation, no locks, no queue access. A stuck worker, poisoned allocator or dead helper costs exactly the budget; the 5 s watchdog is not involved. A fault on the worker or helper thread skips the wait.
• dying flag: claims (claim::try_claim*, which queue and recovery go through), the in-memory drain, the launch-path recovery pass and the worker's wait for it all stop while it is set, so a report is never half-delivered in the death window and a crash inside the launch pass leaves the worker free to answer the flush. Fixes a 5 s UNOWNED_GRACE stall seen in fork_safety.
• Fork: the pid recorded at registration makes the handler skip the flush in a child until its revived recorder re-registers (new pipe + helper; inherited fds closed).
• Panic hook: a main-thread panic no guard will catch runs the same bounded flush (panic = "abort" without the native handler).
• Last registered recorder wins; documented.

Tradeoffs

• Needs a live worker and allocator: a fault that corrupts the heap or hits the worker thread forfeits it.
• Throughput-bound: about 3500 events of ~450 bytes drain in 50 ms on a debug build; the rest of a larger burst is lost.
• No spill file (a producer-side pre-serialized ring would survive a dead worker but taxes every event); no new on-disk format. A cut-short flush leaves a torn part-file tail, which recovery already ignores (pinned by a new test).

Tests

• bugsee-core/tests/crash_flush_e2e.rs: handshake, stuck/panicking flusher, last-registered-wins, pipeline-thread skip, dying flag and claims, a dying process recovers nothing and leaves the session pending, fork child gets its own helper, real pipeline drains everything.
• bugsee-core/tests/recovery_e2e.rs: torn/garbage part tails are ignored.
• bugsee/tests/crash_inflight.rs (subprocess, harness = false, unix + windows): burst with a long budget recovers all 1000; small burst under the real 50 ms budget recovers all 250; wedged worker makes the process wait the (shortened) budget and no more, and still reports. All three fail with the flush disabled. Stable 7/7 runs including under 16 CPU burners.

Not verified

• Windows and Linux runs (developed and tested on macOS arm64); the Windows half of crash_flush.rs was only type-checked standalone, and bugsee-native's Windows path was not compiled locally.
• Drain throughput numbers are from a macOS debug build.

Stack

Stacked on #33 (fix/issue-10) -> #32 (fix/issue-14) -> #24 (fix/issue-9).

Closes #13

🤖 Generated with Claude Code

[cursor]

Stale comment

Deep review: crash-time in-flight flush (#13)

The design is the right shape for a signal-safe best-effort drain: marker first, then one atomic ticket, one non-blocking wake, a pre-created helper that posts the existing FIFO Flush, and a 50 ms cap with no allocation on the faulting thread. The Unix handler path, fork OWNER_PID gate, pipeline-thread skip, and wedged-worker subprocess test all match that contract.

The hole is that Msg::Flush is only consumed in worker_loop. The dying flag was added so a crash inside launch-path recovery would free the worker to answer that message. It only breaks out of the wait sleep; the worker then still walks recovery before it ever recvs.

Overall risk: Medium

The crash marker and frames are still written. What fails is the feature this PR exists for: last breadcrumbs/logs/network events still in the capture channel.

Merge recommendation: request changes

Fix the P1 (skip leftover recovery once is_dying() and enter worker_loop) before merging. The two P2s are small, local, and worth taking in the same pass.

Most important to fix

1. P1 — After the dying wait returns, skip run_recovery / gc_orphans / legacy drains and go straight to worker_loop so the queued Flush can run inside the 50 ms budget.
2. P2 — Do not deregister CrashFlush until the worker has acked Stop (or treat a missing target as “no ack” rather than success).
3. P2 — Windows request_and_wait: integer QPC math, no f64 in the exception filter.

Positive

• Marker-then-flush ordering on every fatal Unix exit and the Windows filter.
• Claims/lenient claims/in-memory drain all refuse new work while dying — that is the right fix for the 5 s UNOWNED_GRACE stall.
• Handshake tests (stuck/panicking helper, last-registered-wins, fork child, torn part tails) plus subprocess burst/wedge cases are the right e2e shape.
• Author already called out that Windows/Linux were not run; the Windows wait path in particular still needs a real exception-filter run.

  

_{Sent by Cursor Automation: Bugsee code review}

[krassx]

Addressed in the latest push: (P1) when dying, the worker skips run_recovery/gc_orphans/legacy drains and enters worker_loop immediately (test: events queued during a dying launch are flushed promptly); (P2) the flush target is dropped only after the worker acks Stop; (P2) Windows wait uses integer QPC math. Windows path still only type-checked. 🤖 Generated with Claude Code

[cursor]

Stale comment

Deep review: crash-time in-flight flush (#13) @ 4893817

Re-review after 4893817 (prior pass was 979c6e8). This HEAD only merges fix/issue-10 (launch-path recovery) and fix/issue-16 (hostinfo) onto the already-reviewed flush; crash_flush.rs, the native/panic call sites, and the e2e tests are unchanged.

Prior findings

• P1 (dying wait still ran run_recovery / gc_orphans) — fully addressed. run_recovery still returns immediately on is_dying() after the merge (heartbeat / skip_launch_pass sit after that gate). The per-session loop, legacy drain, and launch-path pass check the same flag. The new worker_loop launch_deferred recovery goes through the same run_recovery gate, so it cannot start a scan once dying. Msg::Flush is still only consumed in worker_loop.
• P2 (unregister CrashFlush before Stop) — fully addressed. Recorder::drop still sends Stop, waits for the ack (or the 2 s give-up), and only then drops the target.
• P2 (Windows f64 in the exception filter) — fully addressed. request_and_wait still uses integer QPC ticks (elapsed_ms = delta * 1000 / freq).

No new P0–P3 with a distinct failure scenario. In-progress find_pending / recover_one / before_send still occupy the worker the same way a wedged sampler does; that remains the documented stuck-worker tradeoff (50 ms cap, subprocess test), not a leftover of the old “start new recovery after dying” hole. Claims, lenient claims, and drain_memory still refuse new work while dying.

CI (38566841097 at review time): ubuntu-latest build/test/clippy SUCCESS. windows-latest tests succeeded, clippy failed (job log not yet fetchable while macos/ios are still running). I did not run the suite locally.

Overall risk: Low

The crash marker and frames are still written first. The 50 ms drain still reaches worker_loop when a crash hits during the launch pass, including after the issue-10 recovery changes.

Merge recommendation: approve

Land once Windows clippy is green (tests there already passed). Hostinfo (#16) is in this diff only because it was merged into the stack; it is not part of the #13 flush work.

Most important to fix

None remaining from this pass. Residual (not blocking the flush contract): Windows clippy on this HEAD; Windows and Linux still have not been run under a real exception filter / SIGSEGV outside CI (author already noted).

Positive

• Marker-then-flush on every fatal Unix exit and the Windows filter; pipeline-thread skip; fork OWNER_PID gate; dying still stops claims and new recovery.
• The issue-10 merge kept the dying short-circuit on the new heartbeat / bounded-scan / deferred-recovery paths instead of reopening the P1 hole.
• Handshake tests plus subprocess burst/wedge cases still match the best-effort signal-safe contract.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review: crash-time in-flight flush (#13) @ 7ab8a90

Re-review after 7ab8a90 (prior pass was 4893817). The unique delta is the Windows clippy fix: WaitForSingleObject is no longer declared twice with different handle types. The flush contract, dying gates, Drop order, and QPC wait are unchanged from the last pass.

Prior findings

• P1 (dying wait still ran run_recovery / gc_orphans) — fully addressed. run_recovery still returns immediately on is_dying(). The per-session loop, legacy drain, launch-path pass, and worker_loop launch_deferred recovery all hit that gate. Msg::Flush is still only consumed in worker_loop, which the worker now reaches without starting a new scan.
• P2 (unregister CrashFlush before Stop) — fully addressed. Recorder::drop still sends Stop, waits for the ack (or the 2 s give-up), and only then drops the target.
• P2 (Windows f64 in the exception filter) — fully addressed. request_and_wait still uses integer QPC ticks (elapsed_ms = delta * 1000 / freq).

This HEAD (7ab8a90)

The previous Windows clippy failure (38566841097) was clashing_extern_declarations: crash_flush declared WaitForSingleObject(usize, …) and process.rs declared WaitForSingleObject(*mut c_void, …). Both now go through one win32 module (Handle = *mut c_void). The helper thread still carries the handles as usize because raw pointers are not Send; it casts back only at the FFI call. WAIT_OBJECT_0 is 0, so the helper’s exit condition is the same as the old != 0. No other duplicate kernel32 declarations remain in bugsee-core (LockFileEx in claim.rs is unique).

No new P0–P3 with a distinct failure scenario. In-progress find_pending / recover_one / before_send / startup queue::gc_orphans can still occupy the worker past the 50 ms budget; that remains the documented stuck-worker tradeoff (subprocess wedge test), not a leftover of the old “start new recovery after dying” hole.

CI (38569249960): ubuntu-latest, windows-latest, MSRV, android, and ios-sim SUCCESS. macos-latest and ios-device still pending at review. I did not run the suite locally.

Overall risk: Low

The crash marker and frames are still written first. The 50 ms drain still reaches worker_loop when a crash hits during the launch pass. Windows now compiles under -D warnings.

Merge recommendation: approve

Most important to fix

None remaining from this pass.

Positive

• Marker-then-flush on every fatal Unix exit and the Windows filter; pipeline-thread skip; fork OWNER_PID gate; dying still stops claims and new recovery.
• One kernel32 surface is the right way to keep clashing_extern_declarations from coming back the next time a wait or event is added.
• Handshake tests plus subprocess burst/wedge cases still match the best-effort signal-safe contract. Windows CI on this HEAD includes those tests.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review: crash-time in-flight flush (#13) @ 4e42eda

Re-review after 4e42eda (prior pass was 7ab8a90). This HEAD remarges fix/issue-10 @ 66e4f18 (now carrying #14 / #9 / main: CrashBackend, origin, C API, Windows termination, hostinfo). Unique #13 files vs that base are still the same 14; crash_flush.rs, the dying gates, Drop order, QPC wait, panic hook, and e2e tests are unchanged from 7ab8a90. Native lib.rs grew with CrashBackend; the flush calls were ported onto the same fatal exits.

Prior findings

• P1 (dying wait still ran run_recovery / gc_orphans) — fully addressed. run_recovery still returns immediately on is_dying(). The per-session loop, legacy drain, launch-path pass, and worker_loop launch_deferred recovery all hit that gate. Msg::Flush is still only consumed in worker_loop, which the worker reaches without starting a new scan.
• P2 (unregister CrashFlush before Stop) — fully addressed. Recorder::drop still sends Stop, waits for the ack (or the 2 s give-up), and only then drops the target.
• P2 (Windows f64 in the exception filter) — fully addressed. request_and_wait still uses integer QPC ticks (elapsed_ms = delta * 1000 / freq).

This HEAD (4e42eda)

CrashBackend intersection is clean: flush_inflight_events still runs after the marker is complete on Windows on_crash, Unix finish_fatal, and both arms of finish_original_report. The sibling P0 (duplicate [target.'cfg(unix)'.dependencies] / libc) did not land here — 1ecfca8 is in the base; cargo metadata --offline loads. One win32 module still owns WaitForSingleObject.

No new P0–P3 with a distinct failure scenario. In-progress find_pending / recover_one / before_send / startup queue::gc_orphans can still occupy the worker past the 50 ms budget; that remains the documented stuck-worker tradeoff (subprocess wedge test), not a leftover of the old “start new recovery after dying” hole.

CI (38579272645): ubuntu-latest, windows-latest, MSRV, android, and ios-sim SUCCESS. macos-latest, ios-device, and the private-API job still pending at review. I ran cargo metadata --offline (workspace loads); I did not run the test suite locally.

Overall risk: Low

The crash marker and frames are still written first. The 50 ms drain still reaches worker_loop when a crash hits during the launch pass, including after the issue-10 / CrashBackend remarge.

Merge recommendation: approve

Most important to fix

None remaining from this pass.

Positive

• Marker-then-flush on every fatal Unix exit and the Windows filter; pipeline-thread skip; fork OWNER_PID gate; dying still stops claims and new recovery.
• The CrashBackend remarge kept the flush after WRITING is released, so the helper/worker are not blocked on the marker latch.
• Handshake tests plus subprocess burst/wedge cases still match the best-effort signal-safe contract. Ubuntu and Windows CI on this HEAD include those tests.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review: crash-time in-flight flush (#13) @ 0f94b90

Re-review after 0f94b90 (prior pass was 4e42eda). Unique #13 vs fix/issue-10 @ 66e4f18 is still the same 14 files. The new commit is the Windows handshake flake: the helper’s ack wait was 250 ms, shorter than a legitimate drain of a capped queue, so after un-wedge it dropped the ack receiver and never retried.

Prior findings

• P1 (dying wait still ran run_recovery / gc_orphans) — fully addressed. run_recovery still returns immediately on is_dying(). The per-session loop, legacy drain, launch-path pass, and worker_loop launch_deferred recovery all hit that gate. Msg::Flush is still only consumed in worker_loop.
• P2 (unregister CrashFlush before Stop) — fully addressed. Recorder::drop still sends Stop, waits for the ack (or the 2 s give-up), and only then drops the target.
• P2 (Windows f64 in the exception filter) — fully addressed. request_and_wait still uses integer QPC ticks (elapsed_ms = delta * 1000 / freq).

This HEAD (0f94b90)

CRASH_FLUSH_ACK_TIMEOUT is now 10 s. That value only bounds a permanently wedged worker occupying the helper; the crashing thread still polls its own budget (DEFAULT_BUDGET = 50 ms in production, set_inflight_flush_budget_for_test in the subprocess cases). I walked the follow-up-crash case: the first Flush is already in the FIFO, so a longer helper wait cannot make a later fault overtake work the worker has not finished. After a timeout the helper still does not retry that ticket — which is exactly why 250 ms was wrong for any request whose budget (or remaining drain) exceeded it. Production 50 ms crashes were already inside 250 ms; the failure was the unwedge-then-long-budget handshake the e2e actually exercises, and any slow-but-healthy drain of the capped queue.

The stuck-worker test now waits until the sampler reports entry instead of sleeping 150 ms, which is the right pin. Marker-then-flush is unchanged: Windows on_crash, Unix finish_fatal, and both arms of finish_original_report still call flush_inflight_events only after WRITING is released.

No new P0–P3 with a distinct failure scenario. Residual only: e2e upper bounds are now very loose (took < 5s for a 50 ms wait; dying-promptness elapsed < 30s), so they would not catch a 5 s UNOWNED_GRACE stall on the launch-wait path. In-progress find_pending / recover_one / before_send / startup queue::gc_orphans can still occupy the worker past 50 ms — documented stuck-worker tradeoff, not a leftover of the old “start new recovery after dying” hole.

CI (38581617934): all 8 jobs SUCCESS (ubuntu, windows, macos, MSRV, android, ios device + sim, private-API). I ran cargo metadata --offline --no-deps (workspace loads); I did not run the test suite locally.

Overall risk: Low

The crash marker and frames are still written first. The 50 ms drain still reaches worker_loop when a crash hits during the launch pass. The helper now outlasts a slow-but-healthy drain instead of reporting failure and going idle.

Merge recommendation: approve

Most important to fix

None remaining from this pass.

Positive

• Waiting on an observable entered flag, not a 150 ms sleep, is what actually wedges the worker on a slow runner.
• 10 s as a helper cap (not a crash-handler cap) matches the contract: the handler is self-bounding; the helper only has to stay alive long enough to publish the ticket.
• Handshake tests plus subprocess burst/wedge cases still match the best-effort signal-safe contract. Windows CI on this HEAD includes those tests and is green.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Deep review: crash-time in-flight flush (#13) @ 4858ce7

Re-review after 4858ce7 (prior pass was 0f94b90). This HEAD remarges origin/main (the #14 / #10 squashes plus #36 docs). Unique #13 vs main is still the same 14 files; vs 0f94b90 the only unique edits are #36 wording (DESIGN.md 12004, two comments). The flush contract, Drop order, QPC wait, and dying gates are unchanged — but the dying flag itself is a sticky latch, and one Unix fatal path can return to a live process after raising it. That was always in this unique code; previous passes treated finish_fatal as process-ending.

Prior findings

• P1 (dying wait still ran run_recovery / gc_orphans) — fully addressed. run_recovery still returns immediately on is_dying(). The per-session loop, legacy drain, launch-path pass, and worker_loop launch_deferred recovery all hit that gate. Msg::Flush is still only consumed in worker_loop.
• P2 (unregister CrashFlush before Stop) — fully addressed. Recorder::drop still sends Stop, waits for the ack (or the 2 s give-up), and only then drops the target.
• P2 (Windows f64 in the exception filter) — fully addressed. request_and_wait still uses integer QPC ticks (elapsed_ms = delta * 1000 / freq).

Findings

P2 Medium — DYING is latched for the process lifetime, but finish_fatal is not always followed by death.

• Location: crates/bugsee-core/src/crash_flush.rs:210 (flush_for_crash); callers crates/bugsee-native/src/lib.rs:2450 (finish_fatal) and 2702/2724 (finish_original_report). Unix handler at 2861–2868 can return after that without die_of.
• Problem: flush_for_crash stores DYING = true and never clears it except reset_dying_for_test. The module docs describe “the (up to budget) window”; the implementation is a sticky process-wide refuse of try_claim / try_claim_lenient, run_recovery, the launch pass, and drain_memory.
• Impact: A process that survives the handler after this flush stops delivering queued reports and recovering other sessions until it exits. The crash marker for this fault is already on disk; what fails is everything after.
• Scenario: Linux/Android host SIGSEGV/SIGBUS handler with SA_RESETHAND that mprotects a mapped page and does not reinstall. call_host itself installs SIG_DFL for SA_RESETHAND, so host_declined is true. Outcome is RetryOneShot. fatal is true on the first fault (did_nothing is still false until RETRY_LIMIT). finish_fatal → flush_for_crash → return, no die_of. The retry succeeds. Claims then fail for the rest of the run. (macOS does not surface SA_RESETHAND, so that host stays Retry and takes the recovered drop_marker path. Windows on_crash has the same latch if a later filter continues execution.)
• Fix: Treat dying as the wait window, not a process lifetime. Either (a) raise it for the flush and clear it when the Unix handler returns without die_of, or (b) call flush_in_flight from finish_fatal / on_crash and set DYING only on the paths that actually kill (die_of, the SIG_DFL + die_of arm, Windows after Handled(false) if you still want it). Pin with a test that runs flush_for_crash (or the RetryOneShot shape) and then successfully claims / launches recovery without reset_dying_for_test.

No other new P0–P3. Residual (not blocking): in-progress find_pending / recover_one / before_send / startup queue::gc_orphans can still occupy the worker past 50 ms — documented stuck-worker tradeoff, not a leftover of the old “start new recovery after dying” hole. Stack-overflow early complete_report still relies on nested SIGABRT reaching finish_original_report for the flush; the marker itself is already on disk.

CI (38595222834): all 8 jobs SUCCESS (ubuntu, windows, macos, MSRV, android, ios device + sim, private-API). I ran cargo metadata --offline --no-deps (workspace loads); I did not run the test suite locally.

Overall risk: Medium

The crash marker and frames are still written first, and the 50 ms drain still reaches worker_loop when a crash hits during the launch pass. The new issue is a process that does not die after that drain: report delivery then stays off.

Merge recommendation: request changes

Most important to fix

1. P2 — Do not leave DYING set when the Unix handler returns to a live process (finish_fatal without die_of). Clear it on that return, or raise it only on paths that actually terminate.

Positive

• Marker-then-flush on every fatal Unix exit and the Windows filter; pipeline-thread skip; fork OWNER_PID gate; Drop order still matches “registered until the worker has stopped.”
• The main remarge kept the dying short-circuit on heartbeat / bounded-scan / deferred-recovery instead of reopening the original P1 hole.
• Handshake tests plus subprocess burst/wedge cases still match the best-effort signal-safe contract. Desktop CI on this HEAD includes those tests and is green.
• #36 docs pass did not touch flush behaviour.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

P2 Medium — DYING is latched for the process lifetime, but finish_fatal is not always followed by death.

flush_for_crash stores true here and never clears it except reset_dying_for_test. The docs above call this “the (up to budget) window”; try_claim / try_claim_lenient, run_recovery, the launch pass, and drain_memory all refuse work for as long as it stays set.

Unix finish_fatal (lib.rs:2450) always calls this, then the handler at 2861–2868 only die_ofs when did_nothing. The other fatal arm returns. On Linux/Android that includes first-fault RetryOneShot: call_host itself installs SIG_DFL for SA_RESETHAND, so host_declined is true. A host that mprotects a mapped page and does not reinstall then retries successfully. This process keeps running, and queued reports / recovery stay refused until it exits. (macOS does not surface SA_RESETHAND, so that host stays Retry and takes drop_marker. finish_original_report has the same latch before aborting_inside_host returns.)

Fix: raise dying for the wait, then clear it when the handler returns without die_of — or call flush_in_flight from finish_fatal and set DYING only on paths that actually kill. Pin with a claim/recovery after flush_for_crash that does not go through reset_dying_for_test.