diff options
| author | Paul Buetow <paul@buetow.org> | 2026-05-13 10:00:43 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-05-13 10:00:43 +0300 |
| commit | f7ebc44d8b770132904b64996eac50e26945bc94 (patch) | |
| tree | 5818658116842106f3e610ef3804dc7d2499b3a5 /internal | |
| parent | 460e1ba7a7228aa324a2f36b8693951b19866c62 (diff) | |
fix: log swallowed defer mgr.Close errors for BPF resources
Wrap the bare `defer mgr.Close()` calls in runTraceWithContext (ior.go)
and runHeadlessParquet (ior_parquet_sink.go) in a closure that checks
the returned error and logs it via logln, so BPF probe-detach failures
and map-cleanup errors are no longer silently discarded.
bpfModule.Close() has no return value and is unchanged.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Diffstat (limited to 'internal')
| -rw-r--r-- | internal/ior.go | 32 | ||||
| -rw-r--r-- | internal/ior_parquet_sink.go | 14 |
2 files changed, 37 insertions, 9 deletions
diff --git a/internal/ior.go b/internal/ior.go index cddb657..a10f1c6 100644 --- a/internal/ior.go +++ b/internal/ior.go @@ -403,14 +403,22 @@ func configureEventLoopOutput(el *eventLoop, mgr *probemanager.Manager, configur } } -func startTraceShutdownWatcher(ctx context.Context, verbose bool, el *eventLoop, profiling *profilingControl, logln func(...any)) { +// startTraceShutdownWatcher launches a goroutine that waits for ctx to be +// cancelled, then flushes stats and stops profiling. It returns a done channel +// that is closed once the goroutine has finished all cleanup. Callers must +// drain this channel before returning to avoid a goroutine leak when the +// context is cancelled but the caller exits before the goroutine runs. +func startTraceShutdownWatcher(ctx context.Context, verbose bool, el *eventLoop, profiling *profilingControl, logln func(...any)) <-chan struct{} { + done := make(chan struct{}) go func() { + defer close(done) <-ctx.Done() if verbose { fmt.Println(el.stats()) } profiling.stop(logln) }() + return done } // maybePrependFlamegraphConfigure wraps configure so that, when flamegraph @@ -430,9 +438,13 @@ func maybePrependFlamegraphConfigure(cfg flags.Config, configure func(*eventLoop return chainEventLoopConfigure(recordOutput, configure), recorder } -// finaliseTrace waits for profiling to finish, flushes the flamegraph recorder -// if one was created, and logs the total run duration. -func finaliseTrace(recorder *flamegraph.Recorder, profiling *profilingControl, totalDuration time.Duration, logln func(...any)) error { +// finaliseTrace waits for the shutdown-watcher goroutine and profiling to +// finish, flushes the flamegraph recorder if one was created, and logs the +// total run duration. watcherDone must be the channel returned by +// startTraceShutdownWatcher; draining it here prevents a goroutine leak when +// the caller's context is cancelled but the goroutine has not yet exited. +func finaliseTrace(watcherDone <-chan struct{}, recorder *flamegraph.Recorder, profiling *profilingControl, totalDuration time.Duration, logln func(...any)) error { + <-watcherDone <-profiling.done if recorder != nil { if err := recorder.Write(); err != nil { @@ -457,7 +469,13 @@ func runTraceWithContext(parentCtx context.Context, cfg flags.Config, started ch return err } defer bpfModule.Close() - defer mgr.Close() + // mgr.Close() detaches BPF probes and releases kernel resources; log any + // error so that probe-detach failures are not silently discarded. + defer func() { + if err := mgr.Close(); err != nil { + logln("BPF probe manager close error:", err) + } + }() defer releaseBindings() ch, err := setupEventChannel(bpfModule) @@ -485,11 +503,11 @@ func runTraceWithContext(parentCtx context.Context, cfg flags.Config, started ch return err } configureEventLoopOutput(el, mgr, configure) - startTraceShutdownWatcher(ctx, verbose, el, profiling, logln) + watcherDone := startTraceShutdownWatcher(ctx, verbose, el, profiling, logln) startTime := time.Now() el.run(ctx, ch) - return finaliseTrace(recorder, profiling, time.Since(startTime), logln) + return finaliseTrace(watcherDone, recorder, profiling, time.Since(startTime), logln) } func chainEventLoopConfigure(fns ...func(*eventLoop)) func(*eventLoop) { diff --git a/internal/ior_parquet_sink.go b/internal/ior_parquet_sink.go index b2a1439..3b87385 100644 --- a/internal/ior_parquet_sink.go +++ b/internal/ior_parquet_sink.go @@ -102,7 +102,13 @@ func runHeadlessParquet(cfg flags.Config) error { return err } defer bpfModule.Close() - defer mgr.Close() + // mgr.Close() detaches BPF probes and releases kernel resources; log any + // error so that probe-detach failures are not silently discarded. + defer func() { + if err := mgr.Close(); err != nil { + logln("BPF probe manager close error:", err) + } + }() defer releaseBindings() ch, err := setupEventChannel(bpfModule) @@ -135,11 +141,15 @@ func runHeadlessParquet(cfg flags.Config) error { sink := newHeadlessParquetSink(recorder, cancel) configureEventLoopOutput(el, mgr, sink.configure) - startTraceShutdownWatcher(ctx, true, el, profiling, logln) + // startTraceShutdownWatcher returns a done channel that must be drained + // before returning to prevent a goroutine leak when ctx is cancelled but + // the goroutine has not yet exited. + watcherDone := startTraceShutdownWatcher(ctx, true, el, profiling, logln) startTime := time.Now() el.run(ctx, ch) totalDuration := time.Since(startTime) + <-watcherDone <-profiling.done stopErr := recorder.Stop() |
