diff --git a/cmd/grg/main.go b/cmd/grg/main.go index ac627d2..f789af0 100644 --- a/cmd/grg/main.go +++ b/cmd/grg/main.go @@ -25,18 +25,36 @@ import ( ) func main() { - ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) - defer cancel() + os.Exit(realMain()) +} + +// realMain owns the whole run and returns the process exit code. main does +// nothing but hand that code to os.Exit, which runs no deferred functions: +// exiting from inside this function would skip signal cleanup on every +// non-zero exit, and exit code 1 (no match found) is the common case. +func realMain() int { + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + + // Release the signal handler as soon as the first signal cancels ctx. That + // restores the default signal disposition, so a second Ctrl-C terminates + // the process immediately instead of being swallowed by a handler that + // would otherwise stay installed for the whole run. This goroutine cannot + // leak: defer stop() cancels ctx on the normal path too, so <-ctx.Done() + // always returns. + go func() { + <-ctx.Done() + stop() + }() if err := runContext(ctx, os.Args[1:], os.Stdout, os.Stderr); err != nil { ec := exitCodeForError(err) - if ec != 1 { - if !isQuietError(err) { - fmt.Fprintf(os.Stderr, "grg: %v\n", err) - } + if ec != 1 && !isQuietError(err) { + fmt.Fprintf(os.Stderr, "grg: %v\n", err) } - os.Exit(ec) + return ec } + return 0 } func run(args []string) error { @@ -129,35 +147,12 @@ func runContext(ctx context.Context, args []string, stdout, stderr io.Writer) er searchPipeline := search.NewPipeline(reader, matcher, cfg) - type chanExecutor interface { - ExecuteContext(ctx context.Context, occurrences []model.BlobOccurrence) (<-chan *search.BlobResult, <-chan error) - } - type sliceExecutor interface { - ExecuteContext(ctx context.Context, occurrences []model.BlobOccurrence) ([]*search.BlobResult, error) - } - var results []*search.BlobResult - if che, ok := any(searchPipeline).(chanExecutor); ok { - resultsCh, errCh := che.ExecuteContext(ctx, occurrences) - for res := range resultsCh { - results = append(results, res) - } - if err = <-errCh; err != nil { - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || ctx.Err() != nil { - return cancelError{err: err, quiet: cfg.Quiet} - } - return err - } - } else if se, ok := any(searchPipeline).(sliceExecutor); ok { - results, err = se.ExecuteContext(ctx, occurrences) - } else { - if ctxErr := ctx.Err(); ctxErr != nil { - return cancelError{err: ctxErr, quiet: cfg.Quiet} - } - results, err = searchPipeline.Execute(occurrences) + resultsCh, errCh := searchPipeline.ExecuteContext(ctx, occurrences) + for res := range resultsCh { + results = append(results, res) } - - if err != nil { + if err = <-errCh; err != nil { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || ctx.Err() != nil { return cancelError{err: err, quiet: cfg.Quiet} } diff --git a/test/integration/signal_test.go b/test/integration/signal_test.go index 0a27d0b..3bd9f27 100644 --- a/test/integration/signal_test.go +++ b/test/integration/signal_test.go @@ -2,13 +2,23 @@ package integration import ( "fmt" + "os" "os/exec" + "strings" "syscall" "testing" "time" ) // setupWorkloadRepo creates a repository with enough commit history to test process interruption. +// +// Note that this workload is weak on purpose-preserving history: 25 commits of +// 10 tiny files are searched in a few milliseconds, so the three tests below +// almost certainly finish before their signal is delivered, and all of them +// accept exit code 0 as valid. They only prove that a signal does not wedge +// the process. TestSignal_SecondSIGINT_ForceExits carries the real assertion: +// it pins the process in a state where the signal has to do the work, and +// accepts nothing but a non-zero exit. func setupWorkloadRepo(t *testing.T) *TestRepo { t.Helper() repo := NewTestRepo(t) @@ -158,3 +168,130 @@ func TestSignal_ImmediateInterrupt(t *testing.T) { // Succeeded in exiting within timeout } } + +// setupOutputHeavyRepo creates a repository whose search emits far more output +// than a pipe buffer holds (64 KiB on Linux), so a grg process whose stdout is +// never drained is guaranteed to fill the pipe and block in write(2) with +// matches still pending. +func setupOutputHeavyRepo(t *testing.T) *TestRepo { + t.Helper() + repo := NewTestRepo(t) + + for i := 1; i <= 30; i++ { + files := make(map[string]string) + for j := 1; j <= 12; j++ { + var b strings.Builder + for k := 1; k <= 8; k++ { + fmt.Fprintf(&b, "commit %d file %d line %d TOKEN_WORKLOAD_MARKER padding padding padding padding\n", i, j, k) + } + files[fmt.Sprintf("pkg%d/file%d.txt", j, j)] = b.String() + } + repo.Commit(fmt.Sprintf("Commit batch %d", i), files) + } + + return repo +} + +// TestSignal_SecondSIGINT_ForceExits verifies that a second SIGINT terminates +// grg even when the run cannot unwind on its own. +// +// The process is pinned in an uninterruptible write: its stdout is a pipe that +// nobody reads, so once the pipe buffer is full every remaining match line +// blocks in write(2). No context check can rescue that goroutine, which is +// exactly the situation a user hits when grg is piped into a stalled consumer. +// The first SIGINT is therefore necessarily absorbed by the installed handler +// and changes nothing observable. Only releasing that handler once cancellation +// has fired - restoring the default disposition - lets the second SIGINT end +// the process. Without that release, package signal keeps relaying and then +// silently dropping every later signal and the process survives until SIGKILL. +func TestSignal_SecondSIGINT_ForceExits(t *testing.T) { + repo := setupOutputHeavyRepo(t) + bin := getGRGBinary(t) + + // The parent closes its copy of the write end so the child owns the only + // writer, and holds the read end open without ever reading it so the child + // blocks instead of receiving EPIPE. + pr, pw, err := os.Pipe() + if err != nil { + t.Fatalf("failed creating pipe: %v", err) + } + defer pr.Close() + + cmd := exec.Command(bin, "--color=never", "TOKEN_WORKLOAD_MARKER") + cmd.Dir = repo.Dir + cmd.Stdout = pw + + if err := cmd.Start(); err != nil { + _ = pw.Close() + t.Fatalf("failed starting grg subprocess: %v", err) + } + _ = pw.Close() + + done := make(chan error, 1) + go func() { + done <- cmd.Wait() + }() + + // Premise check: the search itself is quick, but the output cannot drain, so + // the process must still be alive and stuck. Had it exited, the workload + // would fit in the pipe buffer and the rest of this test would prove nothing. + select { + case err := <-done: + t.Fatalf("grg exited (%v) before filling the stdout pipe; workload is too small for this test to mean anything", err) + case <-time.After(1 * time.Second): + } + + if err := cmd.Process.Signal(syscall.SIGINT); err != nil { + t.Fatalf("failed sending first SIGINT: %v", err) + } + + // Give cancellation time to propagate and release the signal handler. + select { + case err := <-done: + // Not expected while blocked in write, but a prompt non-zero exit is a + // perfectly good outcome for an interrupted run. + assertInterruptedExit(t, err) + return + case <-time.After(250 * time.Millisecond): + } + + if err := cmd.Process.Signal(syscall.SIGINT); err != nil { + t.Fatalf("failed sending second SIGINT: %v", err) + } + + select { + case <-time.After(2 * time.Second): + _ = cmd.Process.Kill() + t.Fatal("second SIGINT was swallowed: grg was still alive 2s later and only SIGKILL ended it") + case err := <-done: + assertInterruptedExit(t, err) + } +} + +// assertInterruptedExit requires that grg died because it was interrupted: +// killed outright by SIGINT once the default disposition was restored, or +// exited non-zero after handling the cancellation itself. Exit code 0 would +// claim the search completed successfully, which it did not. +func assertInterruptedExit(t *testing.T, err error) { + t.Helper() + + if err == nil { + t.Fatal("grg exited 0 after being interrupted; an interrupted run must not report success") + } + + exitErr, ok := err.(*exec.ExitError) + if !ok { + t.Fatalf("waiting on grg failed: %v", err) + } + + if ws, ok := exitErr.Sys().(syscall.WaitStatus); ok && ws.Signaled() { + if ws.Signal() != syscall.SIGINT { + t.Fatalf("expected termination by SIGINT, got %v", ws.Signal()) + } + return + } + + if code := exitErr.ExitCode(); code == 0 { + t.Fatalf("expected a non-zero exit code after interruption, got %d", code) + } +}