aspect-build / aspect-build/aspect-cli

[Bug]: fatal-signal crash handler produces no report and no stderr marker on a real crash, even with `ASPECT_CRASH_LOG` set (#1419 follow-up)

Open
#1,433 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Starlark
Stars
165
Forks
45
Avg merge
1d 47m
Merged PRs (30d)
35

Description

### Summary

We wired up exactly the follow-up #1419 asked for — the `-debug-` CLI variant with `ASPECT_CRASH_LOG=` on local disk, uploaded as a CI artifact on failure. On the first real SIGSEGV after that landed, the handler produced **nothing**: no crash-log file (not even an empty one), no stderr marker, no partial report. So the hardening in #1419 doesn't close the "zero output" hole it was written for, and the ongoing lint segfault investigation is still blind.

Companion to #1432 (the crash itself).

### Evidence

CLI `v2026.36.5`, `aspect-cli-debug-x86_64-unknown-linux-musl` (static-pie, unstripped), Aspect Workflows self-hosted GHA runner, 2026-09-09 14:06 UTC.

Step env, verbatim from the job log:

```
##[group]Run aspect lint --task:name lint
env:
ASPECT_DEBUG_CLI: 1
ASPECT_CRASH_LOG: /mnt/ephemeral/workdir/_temp/aspect-cli-crash.log
MIMALLOC_SHOW_ERRORS: 1
##[endgroup]
downloading aspect cli version v2026.36.5 file aspect-cli-debug-x86_64-unknown-linux-musl
Aspect CLI v2026.36.5 (debug build) — https://aspect.build/docs/cli
```

Outcome:

```
/mnt/ephemeral/workdir/_temp/.sh: line 1: 50226 Segmentation fault (core dumped) aspect lint --task:name lint
##[error]Process completed with exit code 139.
...
Run actions/upload-artifact@v4
path: /mnt/ephemeral/workdir/_temp/aspect-cli-crash.log
No files were found with the provided path: /mnt/ephemeral/workdir/_temp/aspect-cli-crash.log. No artifacts will be uploaded.
```

`ASPECT_NO_CRASH_HANDLER` was unset. `runner.temp` is ordinary local disk on the ephemeral workdir, not a network mount, so no wedged `open(2)`. The full job stderr contains no `*** aspect-cli` marker either.

### The handler itself is fine — verified by A/B

Downloaded the same artifact locally (`aspect-cli-debug-x86_64-unknown-linux-musl` for `v2026.36.5`, 56,285,376 bytes) and ran

```
ASPECT_CRASH_LOG=/tmp/crash.log ASPECT_INTERNAL_TEST_CRASH= ./aspect-cli-debug --version
```

| mode | exit | stderr report | `ASPECT_CRASH_LOG` |
|---|---|---|---|
| `segv` | 139 | full | 538 bytes |
| `segv-thread` | 139 | full | 538 bytes |
| `stackoverflow-thread` | 139 | full | 549 bytes |

Every one wrote a complete report (signal, fault address, `crash pc` with the ASLR bias removed, the `addr2line` hint) to **both** sinks. So the mechanism works on a static-musl binary, the env var is honored, and a stack overflow on a spawned Rust thread is *not* the silent case.

### What the missing file rules out

Entry order in `crates/aspect-cli/src/crash_handler.rs:494-515` is:

```rust
extern "C" fn handler(sig, info, ctx) {
if HANDLING.swap(true, Ordering::SeqCst) {
reset_and_reraise(sig); // SIG_DFL + raise -> whole process dies now
}
open_crash_log(); // open(O_WRONLY|O_CREAT|O_APPEND)
let log_fd = LOG_FD.load(Relaxed);
if log_fd >= 0 { write_marker(log_fd, sig); }
write_marker(libc::STDERR_FILENO, sig);
...
}
```

`open_crash_log()` passes `O_CREAT`, so a handler that got as far as line 504 leaves a file behind even if it dies immediately after. The file **does not exist**, and between handler entry and that `open(2)` there is only an `AtomicBool::swap` and a `OnceLock::get`. So the handler was almost certainly never entered on the faulting thread.

### Ranked hypotheses

1. **`SA_ONSTACK` with no guaranteed alternate stack.** `install()` (line 162) sets `SA_SIGINFO | SA_ONSTACK`, but the tree contains no `sigaltstack` call at all — it relies entirely on the per-thread altstack Rust std installs for its own stack-overflow guard (which aspect's `sigaction` then overrides the *disposition* of). Any thread not created by `std::thread` — anything spawned inside a C dependency, or any thread whose guard page is hit with an exhausted stack and no altstack — cannot have a signal frame pushed, and the kernel forces the default action: silent SIGSEGV, core dumped, handler never runs. Our local `stackoverflow-thread` A/B passes precisely because that thread *is* a std thread, so the test doesn't cover this case.
2. **The `HANDLING` loser kills the process before the winner writes.** A second thread faulting while the first is inside the handler calls `reset_and_reraise` immediately, which sets `SIG_DFL` and `raise`s — terminating the process. Concurrent faults on several threads are exactly what a corrupted shared heap produces. The window between the winner's `swap` and its `open(2)` is sub-microsecond, so this needs a near-simultaneous fault to explain a *missing* file, which makes it less likely than (1) — but the design flaw is real regardless: **the losing thread writes nothing at all**, so in the common case it silently discards half the story.
3. A fault inside the handler before the first `open(2)`. Only an `AtomicBool::swap` and a `OnceLock::get` live there, so this would require the handler's own code or the `CRASH_LOG_PATH` static to be unmapped. Unlikely, but it is also indistinguishable from (1) in a CI log today.

### Suggested fixes

- **Install an alternate signal stack.** `sigaltstack(2)` on the main thread at `install()`, and on every runtime-spawned thread (tokio's `on_thread_start`), so `SA_ONSTACK` actually has a stack to land on. Without it the flag is a no-op at best and a silent-loss mode at worst.
- **Let the losing thread name the signal.** Write the marker (and `O_CREAT` the crash log) *before* the `HANDLING` check, or have the `reset_and_reraise` path emit its own one-line marker first. `write(2)` of a fixed byte string is async-signal-safe and idempotent enough that a duplicated marker is a far better outcome than silence. Optionally make the loser `pause()`/`nanosleep`-loop briefly instead of re-raising, so the winner gets to finish its report before the process dies.
- **Create the crash log at install time,** not in the handler. Then an absent file means the env var wasn't plumbed through, an empty file means the handler never ran, and a populated file means it ran. Today all three collapse into "no artifact," which is why this crash cost us a day of ambiguity.
- **A `--version`-visible breadcrumb** that the handler is installed and which sinks are armed would let CI assert the instrumentation is live before waiting days for a crash.

Happy to test any of these on our workload — we reproduce the underlying segfault at roughly 2/day and can run a branch build in CI.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in crates/aspect-cli/src/crash_handler.rs, especially install() around line 162 and handler() around lines 494-515; inspect how SA_ONSTACK, HANDLING, and crash-log creation behave on threads without a guaranteed alternate stack. Reproduce the non-std-thread or concurrent-fault cases described in the issue, then verify that a real SIGSEGV always leaves a crash-log file and stderr marker.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli, operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.