diff --git a/cmd/desktop/app.go b/cmd/desktop/app.go index 8da0e6606..4d432796a 100644 --- a/cmd/desktop/app.go +++ b/cmd/desktop/app.go @@ -120,4 +120,7 @@ func (a *DesktopApp) shutdown(ctx context.Context) { stopNativeMouseMonitor() log.Println("Desktop app shutting down...") app.Shutdown(a.srv) + // Last, so a teardown that hangs or is killed still reads as an unclean + // exit — that is precisely the failure worth knowing about. + markSessionEnd() } diff --git a/cmd/desktop/lock_unix.go b/cmd/desktop/lock_unix.go new file mode 100644 index 000000000..17df60310 --- /dev/null +++ b/cmd/desktop/lock_unix.go @@ -0,0 +1,25 @@ +//go:build !windows + +package main + +import ( + "errors" + "os" + + "golang.org/x/sys/unix" +) + +// tryLockFile takes an exclusive lock without blocking. The kernel drops it +// when the process exits, however it exits, which is what lets a later run +// tell a crashed session from a running one. +func tryLockFile(f *os.File) (bool, error) { + err := unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB) + switch { + case err == nil: + return true, nil + case errors.Is(err, unix.EWOULDBLOCK): + return false, nil + default: + return false, err + } +} diff --git a/cmd/desktop/lock_windows.go b/cmd/desktop/lock_windows.go new file mode 100644 index 000000000..2ca4ab76c --- /dev/null +++ b/cmd/desktop/lock_windows.go @@ -0,0 +1,28 @@ +package main + +import ( + "errors" + "os" + + "golang.org/x/sys/windows" +) + +// tryLockFile takes an exclusive lock without blocking. Windows releases it +// when the handle closes, including on process death, which is what lets a +// later run tell a crashed session from a running one. +func tryLockFile(f *os.File) (bool, error) { + var overlapped windows.Overlapped + err := windows.LockFileEx( + windows.Handle(f.Fd()), + windows.LOCKFILE_EXCLUSIVE_LOCK|windows.LOCKFILE_FAIL_IMMEDIATELY, + 0, 1, 0, &overlapped, + ) + switch { + case err == nil: + return true, nil + case errors.Is(err, windows.ERROR_LOCK_VIOLATION), errors.Is(err, windows.ERROR_IO_PENDING): + return false, nil + default: + return false, err + } +} diff --git a/cmd/desktop/main.go b/cmd/desktop/main.go index 6fbc408df..93c3edac8 100644 --- a/cmd/desktop/main.go +++ b/cmd/desktop/main.go @@ -225,6 +225,12 @@ func main() { desktopApp := NewDesktopApp(srv, timelineStoreCfg) + // Record this run and report how the last one ended. Claimed here rather + // than at startup so the checks above, which exit on bad configuration, + // cannot strand a marker and have the next launch report a phantom crash. + markSessionStart() + updater.OnBeforeExit(markSessionEnd) + // Run Wails application err = wails.Run(&options.App{ Title: windowTitle, diff --git a/cmd/desktop/session_marker.go b/cmd/desktop/session_marker.go new file mode 100644 index 000000000..b757aede2 --- /dev/null +++ b/cmd/desktop/session_marker.go @@ -0,0 +1,205 @@ +package main + +import ( + "log" + "os" + "path/filepath" + "strconv" + "strings" + "sync" +) + +// The desktop app cannot log its own segfault, so a bug report has no way to +// say whether the last run crashed or the user simply quit — the journal shows +// a process that stopped either way. +// +// Each run creates a file named after its PID and holds an exclusive lock on +// it for its whole lifetime. The lock is what carries the signal, not the file: +// the kernel releases it when the process dies, however it dies, so a marker +// we can lock belonged to a run that is gone. A marker we cannot lock belongs +// to an instance that is still alive. +// +// Liveness is deliberately not inferred from the PID. PIDs are reused, and a +// recycled one would make a crashed run look like a running second window — +// suppressing a real crash and reporting a window that does not exist. +// +// One file per PID rather than a single shared one: a second window must never +// adopt or delete the first one's marker, or quitting either would erase the +// other's crash evidence. +// +// Markers are scoped per host. A home directory can be shared across machines, +// and a lock taken on one host says nothing about a process on another — a +// flat directory would let one machine report a session running happily +// elsewhere as a crash. +func sessionDir() string { + home, err := os.UserHomeDir() + if err != nil { + return "" + } + return filepath.Join(home, ".radar", "desktop-sessions", hostSlug()) +} + +func hostSlug() string { + name, err := os.Hostname() + if err != nil || name == "" { + return "unknown-host" + } + // Keep it a single safe path element regardless of what the OS reports. + name = strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '-', r == '_', r == '.': + return r + default: + return '-' + } + }, name) + if name = strings.Trim(name, "."); name == "" { + return "unknown-host" + } + return name +} + +// runningMarker is written under the lock once a run is fully recorded. A +// marker without it is either mid-creation or already released, and neither is +// a crash — the file existing is not enough, because it is briefly visible and +// unlocked at both ends of a run. +const runningMarker = "running" + +// held keeps this run's marker open. Closing the file would drop the lock and +// advertise the process as gone while it is still running. +// +// Guarded because release is reachable from two goroutines: the Wails shutdown +// callback, and the self-update relaunch. Closing the window while a relaunch +// is pending runs both. +var ( + sessionMu sync.Mutex + held *os.File +) + +func markSessionStart() { claimSession(sessionDir()) } +func markSessionEnd() { releaseSession(sessionDir()) } + +// claimSession reports any run that ended without cleaning up, then records +// this one. It must be called only once the process is committed to running: +// claiming before the startup checks would leave a marker behind on every +// os.Exit and report the next launch as a crash that never happened. +func claimSession(dir string) { + sessionMu.Lock() + defer sessionMu.Unlock() + + if dir == "" || held != nil { + return + } + + // Scan before claiming, so this run's own marker is never mistaken for an + // abandoned one. + reportAbandonedSessions(dir) + + if err := os.MkdirAll(dir, 0o700); err != nil { + log.Printf("[desktop] could not record session marker: %v", err) + return + } + + path := sessionFile(dir, os.Getpid()) + file, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + log.Printf("[desktop] could not record session marker: %v", err) + return + } + locked, err := tryLockFile(file) + if err != nil || !locked { + // Without the lock the marker would claim this run had already ended. + if err != nil { + log.Printf("[desktop] could not lock session marker: %v", err) + } + file.Close() + _ = os.Remove(path) + return + } + + // Only now, holding the lock, does the marker mean "a run is in progress". + if _, err := file.WriteString(runningMarker); err != nil { + log.Printf("[desktop] could not record session marker: %v", err) + file.Close() + _ = os.Remove(path) + return + } + held = file +} + +// releaseSession drops this run's marker so a deliberate exit is not reported +// as a crash. Other instances' markers are left alone. +func releaseSession(dir string) { + sessionMu.Lock() + defer sessionMu.Unlock() + + if dir == "" || held == nil { + return + } + // Clear the marker while the lock is still held. Unlinking first would + // leave the path briefly present and lockable, and a launch landing in + // that gap would report this deliberate quit as a crash. + if err := held.Truncate(0); err != nil { + log.Printf("[desktop] could not clear session marker: %v", err) + } + held.Close() // releases the lock + held = nil + if err := os.Remove(sessionFile(dir, os.Getpid())); err != nil && !os.IsNotExist(err) { + log.Printf("[desktop] could not clear session marker: %v", err) + } +} + +// reportAbandonedSessions logs every marker whose owner is gone and clears it, +// so one crash is reported once rather than on every launch afterwards. +func reportAbandonedSessions(dir string) { + entries, err := os.ReadDir(dir) + if err != nil { + return + } + + for _, entry := range entries { + pid, err := strconv.Atoi(entry.Name()) + if err != nil || pid <= 0 { + continue + } + + path := filepath.Join(dir, entry.Name()) + file, err := os.OpenFile(path, os.O_RDWR, 0o600) + if err != nil { + continue + } + locked, err := tryLockFile(file) + if err != nil || !locked { + // Held by a live instance, or the filesystem cannot lock. Saying + // nothing beats guessing at a crash that may not have happened. + file.Close() + continue + } + + // Lockable and still marked running: the owner died without clearing + // it. Anything else is a released or half-written marker, which is + // swept away without a claim in either direction. + if markedRunning(file) { + log.Printf("[desktop] previous run (pid %d) did not exit cleanly — it crashed or was force-quit", pid) + } + file.Close() + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + log.Printf("[desktop] could not clear stale session marker: %v", err) + } + } +} + +func sessionFile(dir string, pid int) string { + return filepath.Join(dir, strconv.Itoa(pid)) +} + +// markedRunning reports whether a marker was fully recorded by a run that then +// never released it. +func markedRunning(f *os.File) bool { + buf := make([]byte, len(runningMarker)) + n, err := f.ReadAt(buf, 0) + if err != nil && n != len(runningMarker) { + return false + } + return string(buf[:n]) == runningMarker +} diff --git a/cmd/desktop/session_marker_test.go b/cmd/desktop/session_marker_test.go new file mode 100644 index 000000000..054870e55 --- /dev/null +++ b/cmd/desktop/session_marker_test.go @@ -0,0 +1,390 @@ +package main + +import ( + "bytes" + "log" + "os" + "path/filepath" + "strconv" + "strings" + "sync" + "testing" +) + +func captureLog(t *testing.T, fn func()) string { + t.Helper() + var buf bytes.Buffer + flags := log.Flags() + log.SetOutput(&buf) + log.SetFlags(0) + t.Cleanup(func() { + log.SetOutput(os.Stderr) + log.SetFlags(flags) + }) + fn() + return buf.String() +} + +// newSessionDir isolates each test and resets the process-wide marker handle, +// which claimSession would otherwise carry between tests. +func newSessionDir(t *testing.T) string { + t.Helper() + if held != nil { + held.Close() + held = nil + } + t.Cleanup(func() { + if held != nil { + held.Close() + held = nil + } + }) + return filepath.Join(t.TempDir(), "desktop-sessions") +} + +// abandonedMarker is what a crashed run leaves: a marker recorded as running, +// with no lock on it, because the kernel released the lock when the process +// died. +func abandonedMarker(t *testing.T, dir string, pid int) { + t.Helper() + writeMarker(t, dir, pid, runningMarker) +} + +func writeMarker(t *testing.T, dir string, pid int, content string) { + t.Helper() + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatalf("prepare session dir: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, strconv.Itoa(pid)), []byte(content), 0o600); err != nil { + t.Fatalf("write marker: %v", err) + } +} + +// liveMarker is what a running instance leaves: a marker held under lock. The +// lock lives on this file handle, so a second attempt to take it fails the way +// it would across processes. +func liveMarker(t *testing.T, dir string, pid int) { + t.Helper() + abandonedMarker(t, dir, pid) + file, err := os.OpenFile(filepath.Join(dir, strconv.Itoa(pid)), os.O_RDWR, 0o600) + if err != nil { + t.Fatalf("open marker: %v", err) + } + locked, err := tryLockFile(file) + if err != nil || !locked { + file.Close() + t.Skipf("filesystem does not support locking here (locked=%v err=%v)", locked, err) + } + t.Cleanup(func() { file.Close() }) +} + +func markerExists(t *testing.T, dir string, pid int) bool { + t.Helper() + _, err := os.Stat(filepath.Join(dir, strconv.Itoa(pid))) + return err == nil +} + +func TestClaimSessionReportsUncleanExit(t *testing.T) { + dir := newSessionDir(t) + abandonedMarker(t, dir, 4242) + + out := captureLog(t, func() { claimSession(dir) }) + + if !strings.Contains(out, "did not exit cleanly") { + t.Errorf("log = %q, want an unclean-exit report", out) + } + if markerExists(t, dir, 4242) { + t.Error("stale marker survived; the crash would be reported again on every later launch") + } + if !markerExists(t, dir, os.Getpid()) { + t.Error("this run did not record a marker") + } +} + +// A crash must be reported once, not on every launch forever afterwards. +func TestClaimSessionReportsAnUncleanExitOnlyOnce(t *testing.T) { + dir := newSessionDir(t) + abandonedMarker(t, dir, 4242) + + captureLog(t, func() { claimSession(dir) }) + releaseSession(dir) + out := captureLog(t, func() { claimSession(dir) }) + + if strings.Contains(out, "did not exit cleanly") { + t.Errorf("log = %q, want the crash reported only on the first launch after it", out) + } +} + +// A marker under lock belongs to a running instance. Claiming a crash there +// would fire every time someone opens a second window. +func TestClaimSessionIgnoresALiveInstance(t *testing.T) { + dir := newSessionDir(t) + liveMarker(t, dir, 4243) + + out := captureLog(t, func() { claimSession(dir) }) + + if strings.Contains(out, "did not exit cleanly") { + t.Errorf("log = %q, want no crash claim while another instance holds its marker", out) + } + if !markerExists(t, dir, 4243) { + t.Error("a running instance's marker was removed; its crash would go unreported") + } +} + +// The whole point of locking rather than checking the PID: a recycled PID must +// not make a crashed run look like a running one. The marker is abandoned, so +// it is reported regardless of what that number now refers to. +func TestClaimSessionReportsCrashUnderAReusedPID(t *testing.T) { + dir := newSessionDir(t) + // A live, unrelated process holding this PID — represented here by the + // only PID guaranteed to be alive during the test. + abandonedMarker(t, dir, os.Getpid()) + + out := captureLog(t, func() { claimSession(dir) }) + + if !strings.Contains(out, "did not exit cleanly") { + t.Errorf("log = %q, want the crash reported even though the PID is live", out) + } + if !markerExists(t, dir, os.Getpid()) { + t.Error("this run did not record a marker after clearing the stale one") + } +} + +// Quitting one window must leave a concurrent instance's marker intact. +func TestReleaseSessionPreservesAnotherInstanceMarker(t *testing.T) { + dir := newSessionDir(t) + liveMarker(t, dir, 4244) + claimSession(dir) + + releaseSession(dir) + + if markerExists(t, dir, os.Getpid()) { + t.Error("own marker survived a clean shutdown") + } + if !markerExists(t, dir, 4244) { + t.Error("a concurrent instance's marker was deleted by this instance quitting") + } +} + +func TestClaimSessionSilentOnFirstRun(t *testing.T) { + dir := newSessionDir(t) + + out := captureLog(t, func() { claimSession(dir) }) + + if out != "" { + t.Errorf("log = %q, want silence when no previous run is recorded", out) + } + if !markerExists(t, dir, os.Getpid()) { + t.Error("first run did not record a marker") + } +} + +// A clean shutdown must leave nothing behind, or every deliberate quit is +// reported as a crash on the next launch. +func TestReleaseSessionClearsOwnMarker(t *testing.T) { + dir := newSessionDir(t) + claimSession(dir) + + releaseSession(dir) + + if markerExists(t, dir, os.Getpid()) { + t.Error("marker still present after shutdown") + } + out := captureLog(t, func() { claimSession(dir) }) + if strings.Contains(out, "did not exit cleanly") { + t.Errorf("log = %q, want a clean shutdown to leave no crash report", out) + } +} + +// The marker only means anything while this run holds its lock. If the lock +// were dropped, a concurrent launch would read the run as already finished. +func TestClaimSessionHoldsTheLockForTheRun(t *testing.T) { + dir := newSessionDir(t) + claimSession(dir) + + other, err := os.OpenFile(sessionFile(dir, os.Getpid()), os.O_RDWR, 0o600) + if err != nil { + t.Fatalf("open own marker: %v", err) + } + defer other.Close() + + locked, err := tryLockFile(other) + if err != nil { + t.Fatalf("lock attempt failed: %v", err) + } + if locked { + t.Error("own marker was lockable mid-run; a concurrent launch would report this run as crashed") + } +} + +func TestClaimSessionIgnoresUnrelatedFiles(t *testing.T) { + dir := newSessionDir(t) + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatalf("prepare session dir: %v", err) + } + for _, name := range []string{"not-a-pid", "-1", "0", ".hidden"} { + if err := os.WriteFile(filepath.Join(dir, name), nil, 0o600); err != nil { + t.Fatalf("write %q: %v", name, err) + } + } + + out := captureLog(t, func() { claimSession(dir) }) + + if out != "" { + t.Errorf("log = %q, want non-PID entries ignored silently", out) + } + if !markerExists(t, dir, os.Getpid()) { + t.Error("this run did not record a marker") + } +} + +func TestSessionDirectoryIsPrivate(t *testing.T) { + dir := newSessionDir(t) + claimSession(dir) + + info, err := os.Stat(dir) + if err != nil { + t.Fatalf("stat session dir: %v", err) + } + if perm := info.Mode().Perm(); perm != 0o700 { + t.Errorf("session dir mode = %o, want 700", perm) + } +} + +func TestSessionHelpersTolerateNoHomeDirectory(t *testing.T) { + // sessionDir returns "" when the home directory is unknown; the helpers + // must degrade to doing nothing rather than panicking. + newSessionDir(t) + claimSession("") + releaseSession("") +} + +// A run is briefly visible and unlocked at both ends of its life: after the +// file is created but before it is locked, and after the lock is dropped but +// before the file is unlinked. A launch landing in either gap can take the +// lock, so the file existing cannot be what marks a crash. +func TestClaimSessionIgnoresAMarkerLeftFromAReleasedRun(t *testing.T) { + dir := newSessionDir(t) + writeMarker(t, dir, 4245, "") // released: truncated before the lock was dropped + + out := captureLog(t, func() { claimSession(dir) }) + + if strings.Contains(out, "did not exit cleanly") { + t.Errorf("log = %q, want a deliberate quit not reported as a crash", out) + } + if markerExists(t, dir, 4245) { + t.Error("released marker was left behind; it would accumulate forever") + } +} + +func TestClaimSessionIgnoresAHalfWrittenMarker(t *testing.T) { + dir := newSessionDir(t) + writeMarker(t, dir, 4246, "run") // created and locked, not yet fully recorded + + out := captureLog(t, func() { claimSession(dir) }) + + if strings.Contains(out, "did not exit cleanly") { + t.Errorf("log = %q, want a half-written marker not reported as a crash", out) + } +} + +// The marker only counts as a crash once the run is fully recorded, so the +// content has to be written while the lock is held. +func TestClaimSessionRecordsTheRunningMarkerUnderLock(t *testing.T) { + dir := newSessionDir(t) + claimSession(dir) + + data, err := os.ReadFile(sessionFile(dir, os.Getpid())) + if err != nil { + t.Fatalf("read own marker: %v", err) + } + if string(data) != runningMarker { + t.Errorf("marker content = %q, want %q", data, runningMarker) + } +} + +// Simulates the release window directly: this run's own marker, cleared while +// still locked, must not read as a crash to the launch that picks it up. +func TestReleasedMarkerIsNotACrashEvenIfUnlinkFails(t *testing.T) { + dir := newSessionDir(t) + claimSession(dir) + + if err := held.Truncate(0); err != nil { + t.Fatalf("truncate own marker: %v", err) + } + held.Close() + held = nil + // Deliberately skip the unlink, standing in for a failed os.Remove. + + out := captureLog(t, func() { claimSession(dir) }) + + if strings.Contains(out, "did not exit cleanly") { + t.Errorf("log = %q, want a cleared marker never reported as a crash", out) + } +} + +// A home directory can be shared across machines. A lock taken on one host +// says nothing about a process on another, so one machine must never read +// another's markers and report a live session as a crash. +func TestSessionDirIsScopedPerHost(t *testing.T) { + dir := sessionDir() + if dir == "" { + t.Skip("no home directory available") + } + if filepath.Base(dir) != hostSlug() { + t.Errorf("session dir = %q, want it scoped under host %q", dir, hostSlug()) + } + if filepath.Base(filepath.Dir(dir)) != "desktop-sessions" { + t.Errorf("session dir = %q, want it under desktop-sessions/", dir) + } +} + +func TestHostSlugIsASafePathElement(t *testing.T) { + slug := hostSlug() + if slug == "" { + t.Fatal("hostSlug() = empty, want a usable path element") + } + if strings.ContainsAny(slug, `/\`) || slug == "." || slug == ".." { + t.Errorf("hostSlug() = %q, want a single safe path element", slug) + } +} + +// Release is reachable from the Wails shutdown callback and from the +// self-update relaunch goroutine; closing the window mid-relaunch runs both. +func TestSessionStateIsSafeUnderConcurrentRelease(t *testing.T) { + dir := newSessionDir(t) + claimSession(dir) + + var wg sync.WaitGroup + for i := 0; i < 16; i++ { + wg.Add(1) + go func() { + defer wg.Done() + releaseSession(dir) + }() + } + wg.Wait() + + if markerExists(t, dir, os.Getpid()) { + t.Error("marker survived concurrent release") + } +} + +// Claim and release can also overlap across goroutines without corrupting the +// handle or leaving a marker behind. +func TestSessionStateIsSafeUnderConcurrentClaimAndRelease(t *testing.T) { + dir := newSessionDir(t) + + var wg sync.WaitGroup + for i := 0; i < 16; i++ { + wg.Add(2) + go func() { defer wg.Done(); claimSession(dir) }() + go func() { defer wg.Done(); releaseSession(dir) }() + } + wg.Wait() + + releaseSession(dir) + if markerExists(t, dir, os.Getpid()) { + t.Error("marker left behind after the final release") + } +} diff --git a/internal/updater/apply_darwin.go b/internal/updater/apply_darwin.go index ab7fb9eac..a5765ee1b 100644 --- a/internal/updater/apply_darwin.go +++ b/internal/updater/apply_darwin.go @@ -97,6 +97,7 @@ func Relaunch() error { return fmt.Errorf("relaunch: %w", err) } + runBeforeExit() os.Exit(0) return nil // unreachable } diff --git a/internal/updater/apply_linux.go b/internal/updater/apply_linux.go index b35941294..93af51c39 100644 --- a/internal/updater/apply_linux.go +++ b/internal/updater/apply_linux.go @@ -96,6 +96,7 @@ func Relaunch() error { return fmt.Errorf("relaunch: %w", err) } + runBeforeExit() os.Exit(0) return nil // unreachable } diff --git a/internal/updater/apply_windows.go b/internal/updater/apply_windows.go index ddd40c07d..c509b625a 100644 --- a/internal/updater/apply_windows.go +++ b/internal/updater/apply_windows.go @@ -90,6 +90,7 @@ func Relaunch() error { return fmt.Errorf("start trampoline: %w", err) } + runBeforeExit() os.Exit(0) return nil // unreachable } diff --git a/internal/updater/before_exit.go b/internal/updater/before_exit.go new file mode 100644 index 000000000..ae78a2fd3 --- /dev/null +++ b/internal/updater/before_exit.go @@ -0,0 +1,40 @@ +package updater + +import "sync" + +// Relaunch ends in os.Exit, which runs no deferred function and no shutdown +// hook, so anything that must happen on a deliberate exit has to be registered +// here. Relaunch is called from its own goroutine while registration happens on +// the startup path, so the slice is guarded rather than left bare. +var ( + beforeExitMu sync.Mutex + beforeExit []func() + beforeExitOnce sync.Once +) + +// OnBeforeExit registers cleanup to run immediately before a self-update +// relaunch terminates this process. +func OnBeforeExit(fn func()) { + if fn == nil { + return + } + beforeExitMu.Lock() + defer beforeExitMu.Unlock() + beforeExit = append(beforeExit, fn) +} + +// runBeforeExit invokes the registered cleanup once. Running twice would let a +// retried relaunch repeat side effects that are only safe to perform on the +// way out. +func runBeforeExit() { + beforeExitOnce.Do(func() { + beforeExitMu.Lock() + callbacks := make([]func(), len(beforeExit)) + copy(callbacks, beforeExit) + beforeExitMu.Unlock() + + for _, fn := range callbacks { + fn() + } + }) +} diff --git a/internal/updater/before_exit_test.go b/internal/updater/before_exit_test.go new file mode 100644 index 000000000..7e82de900 --- /dev/null +++ b/internal/updater/before_exit_test.go @@ -0,0 +1,89 @@ +package updater + +import ( + "sync" + "testing" +) + +// resetBeforeExit clears the process-wide hook state so each test starts from +// the same place; sync.Once cannot be reset, so it is replaced outright. +func resetBeforeExit(t *testing.T) { + t.Helper() + beforeExitMu.Lock() + original := beforeExit + beforeExit = nil + beforeExitOnce = sync.Once{} + beforeExitMu.Unlock() + + t.Cleanup(func() { + beforeExitMu.Lock() + beforeExit = original + beforeExitOnce = sync.Once{} + beforeExitMu.Unlock() + }) +} + +// Relaunch ends in os.Exit, which runs no deferred function and no shutdown +// hook. Anything that must happen on a deliberate exit only happens if these +// callbacks fire, in the order they were registered. +func TestRunBeforeExitInvokesRegisteredCleanup(t *testing.T) { + resetBeforeExit(t) + + var order []string + OnBeforeExit(func() { order = append(order, "first") }) + OnBeforeExit(func() { order = append(order, "second") }) + + runBeforeExit() + + if len(order) != 2 || order[0] != "first" || order[1] != "second" { + t.Errorf("callbacks ran as %v, want [first second]", order) + } +} + +// A retried relaunch must not repeat cleanup that is only safe on the way out. +func TestRunBeforeExitRunsOnlyOnce(t *testing.T) { + resetBeforeExit(t) + + calls := 0 + OnBeforeExit(func() { calls++ }) + + runBeforeExit() + runBeforeExit() + + if calls != 1 { + t.Errorf("callback ran %d times, want 1", calls) + } +} + +func TestRunBeforeExitWithNoneRegistered(t *testing.T) { + resetBeforeExit(t) + runBeforeExit() +} + +func TestOnBeforeExitIgnoresNil(t *testing.T) { + resetBeforeExit(t) + + OnBeforeExit(nil) + runBeforeExit() // must not panic +} + +// Relaunch runs on its own goroutine while registration happens on the startup +// path, so both sides have to be safe under -race. +func TestOnBeforeExitIsSafeUnderConcurrentUse(t *testing.T) { + resetBeforeExit(t) + + var wg sync.WaitGroup + for i := 0; i < 32; i++ { + wg.Add(1) + go func() { + defer wg.Done() + OnBeforeExit(func() {}) + }() + } + wg.Add(1) + go func() { + defer wg.Done() + runBeforeExit() + }() + wg.Wait() +}