From e05a89e0b820c9f4e423b25ecf435b82235f59eb Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 19 Nov 2024 18:40:00 -0800 Subject: [PATCH] improve stacks of cancels from defers In this case the current stack trace points to the line where the context was created. Instead the stack should be captured when the defer is running so the return path to the defer call is also part of the stack. Signed-off-by: Tonis Tiigi --- cache/remotecache/local/local.go | 2 +- client/build_test.go | 2 +- client/client_test.go | 2 +- cmd/buildctl/common/common.go | 2 +- cmd/buildkitd/main.go | 2 +- control/gateway/gateway.go | 2 +- executor/runcexecutor/executor.go | 4 ++-- exporter/local/export.go | 2 +- exporter/oci/export.go | 2 +- exporter/tar/export.go | 2 +- frontend/dockerfile/dockerfile_test.go | 2 +- session/filesync/filesync.go | 2 +- session/group.go | 2 +- session/manager.go | 4 ++-- session/session.go | 2 +- solver/jobs.go | 2 +- solver/llbsolver/solver.go | 2 +- solver/scheduler.go | 2 +- solver/scheduler_test.go | 2 +- source/containerimage/ocilayout.go | 2 +- source/local/source.go | 2 +- util/flightcontrol/flightcontrol.go | 4 ++-- util/testutil/integration/registry.go | 2 +- util/testutil/integration/run.go | 2 +- util/tracing/otlptracegrpc/client.go | 2 +- 25 files changed, 28 insertions(+), 28 deletions(-) diff --git a/cache/remotecache/local/local.go b/cache/remotecache/local/local.go index 97db913ba..950842763 100644 --- a/cache/remotecache/local/local.go +++ b/cache/remotecache/local/local.go @@ -107,7 +107,7 @@ func getContentStore(ctx context.Context, sm *session.Manager, g session.Group, } timeoutCtx, cancel := context.WithCancelCause(ctx) timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() caller, err := sm.Get(timeoutCtx, sessionID, false) if err != nil { diff --git a/client/build_test.go b/client/build_test.go index f8fb1df27..10e746109 100644 --- a/client/build_test.go +++ b/client/build_test.go @@ -1236,7 +1236,7 @@ func testClientGatewayContainerCancelExecTty(t *testing.T, sb integration.Sandbo defer ctr.Release(ctx) execCtx, cancel := context.WithCancelCause(ctx) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() prompt := newTestPrompt(execCtx, t, inputW, output) pid2, err := ctr.Start(execCtx, client.StartRequest{ diff --git a/client/client_test.go b/client/client_test.go index c801eea31..ab2e3d1cc 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -8018,7 +8018,7 @@ func testInvalidExporter(t *testing.T, sb integration.Sandbox) { func testParallelLocalBuilds(t *testing.T, sb integration.Sandbox) { integration.SkipOnPlatform(t, "windows") ctx, cancel := context.WithCancelCause(sb.Context()) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() c, err := New(ctx, sb.Address()) require.NoError(t, err) diff --git a/cmd/buildctl/common/common.go b/cmd/buildctl/common/common.go index bae721079..48a05e494 100644 --- a/cmd/buildctl/common/common.go +++ b/cmd/buildctl/common/common.go @@ -86,7 +86,7 @@ func ResolveClient(c *cli.Context) (*client.Client, error) { ctx2, cancel := context.WithCancelCause(ctx) ctx2, _ = context.WithTimeoutCause(ctx2, timeout*time.Second, errors.WithStack(context.DeadlineExceeded)) ctx = ctx2 - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() } cl, err := client.New(ctx, c.GlobalString("addr"), opts...) diff --git a/cmd/buildkitd/main.go b/cmd/buildkitd/main.go index 523c60510..7bfd8f7d9 100644 --- a/cmd/buildkitd/main.go +++ b/cmd/buildkitd/main.go @@ -233,7 +233,7 @@ func main() { return errors.New("rootless mode requires to be executed as the mapped root in a user namespace; you may use RootlessKit for setting up the namespace") } ctx, cancel := context.WithCancelCause(appcontext.Context()) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() cfg, err := config.LoadFile(c.GlobalString("config")) if err != nil { diff --git a/control/gateway/gateway.go b/control/gateway/gateway.go index 3012c1e26..5b51d252f 100644 --- a/control/gateway/gateway.go +++ b/control/gateway/gateway.go @@ -61,7 +61,7 @@ func (gwf *GatewayForwarder) lookupForwarder(ctx context.Context) (gateway.LLBBr ctx, cancel := context.WithCancelCause(ctx) ctx, _ = context.WithTimeoutCause(ctx, 3*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() go func() { <-ctx.Done() diff --git a/executor/runcexecutor/executor.go b/executor/runcexecutor/executor.go index 49e12c285..f1a8eab83 100644 --- a/executor/runcexecutor/executor.go +++ b/executor/runcexecutor/executor.go @@ -547,7 +547,7 @@ func (k procKiller) Kill(ctx context.Context) (err error) { // shorter timeout but here as a fail-safe for future refactoring. ctx, cancel := context.WithCancelCause(ctx) ctx, _ = context.WithTimeoutCause(ctx, 10*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() if k.pidfile == "" { // for `runc run` process we use `runc kill` to terminate the process @@ -694,7 +694,7 @@ func (p *procHandle) WaitForReady(ctx context.Context) error { func (p *procHandle) WaitForStart(ctx context.Context, startedCh <-chan int, started func()) error { ctx, cancel := context.WithCancelCause(ctx) ctx, _ = context.WithTimeoutCause(ctx, 10*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() select { case <-ctx.Done(): return errors.New("go-runc started message never received") diff --git a/exporter/local/export.go b/exporter/local/export.go index a5228cb33..225503b3d 100644 --- a/exporter/local/export.go +++ b/exporter/local/export.go @@ -81,7 +81,7 @@ func (e *localExporter) Config() *exporter.Config { func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source, _ exptypes.InlineCache, sessionID string) (map[string]string, exporter.DescriptorReference, error) { timeoutCtx, cancel := context.WithCancelCause(ctx) timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() if e.opts.Epoch == nil { if tm, ok, err := epoch.ParseSource(inp); err != nil { diff --git a/exporter/oci/export.go b/exporter/oci/export.go index 93fe9265f..df8e4134e 100644 --- a/exporter/oci/export.go +++ b/exporter/oci/export.go @@ -218,7 +218,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src *exporter.Source timeoutCtx, cancel := context.WithCancelCause(ctx) timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() caller, err := e.opt.SessionManager.Get(timeoutCtx, sessionID, false) if err != nil { diff --git a/exporter/tar/export.go b/exporter/tar/export.go index 1b6f36f77..96eedffe8 100644 --- a/exporter/tar/export.go +++ b/exporter/tar/export.go @@ -165,7 +165,7 @@ func (e *localExporterInstance) Export(ctx context.Context, inp *exporter.Source timeoutCtx, cancel := context.WithCancelCause(ctx) timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() caller, err := e.opt.SessionManager.Get(timeoutCtx, sessionID, false) if err != nil { diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index d8f458192..e777690d5 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -3493,7 +3493,7 @@ COPY . . ctx, cancel := context.WithCancelCause(sb.Context()) ctx, _ = context.WithTimeoutCause(ctx, 15*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() c, err := client.New(ctx, sb.Address()) require.NoError(t, err) diff --git a/session/filesync/filesync.go b/session/filesync/filesync.go index 4bdb25f02..4c7e4b233 100644 --- a/session/filesync/filesync.go +++ b/session/filesync/filesync.go @@ -206,7 +206,7 @@ func FSSync(ctx context.Context, c session.Caller, opt FSSendRequestOpt) error { opts[keyDirName] = []string{opt.Name} ctx, cancel := context.WithCancelCause(ctx) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() client := NewFileSyncClient(c.Conn()) diff --git a/session/group.go b/session/group.go index e701d1d29..66d18fcc7 100644 --- a/session/group.go +++ b/session/group.go @@ -74,7 +74,7 @@ func (sm *Manager) Any(ctx context.Context, g Group, f func(context.Context, str timeoutCtx, cancel := context.WithCancelCause(ctx) timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() c, err := sm.Get(timeoutCtx, id, false) if err != nil { lastErr = err diff --git a/session/manager.go b/session/manager.go index 02383dc09..f411f0a3b 100644 --- a/session/manager.go +++ b/session/manager.go @@ -99,7 +99,7 @@ func (sm *Manager) HandleConn(ctx context.Context, conn net.Conn, opts map[strin // caller needs to take lock, this function will release it func (sm *Manager) handleConn(ctx context.Context, conn net.Conn, opts map[string][]string) error { ctx, cancel := context.WithCancelCause(ctx) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() opts = canonicalHeaders(opts) @@ -154,7 +154,7 @@ func (sm *Manager) Get(ctx context.Context, id string, noWait bool) (Caller, err } ctx, cancel := context.WithCancelCause(ctx) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() go func() { <-ctx.Done() diff --git a/session/session.go b/session/session.go index bb2bccccb..b67e25ece 100644 --- a/session/session.go +++ b/session/session.go @@ -96,7 +96,7 @@ func (s *Session) Run(ctx context.Context, dialer Dialer) error { s.cancelCtx = cancel s.done = make(chan struct{}) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() defer close(s.done) meta := make(map[string][]string) diff --git a/solver/jobs.go b/solver/jobs.go index 5b1e12198..d8cce3abd 100644 --- a/solver/jobs.go +++ b/solver/jobs.go @@ -626,7 +626,7 @@ func (jl *Solver) NewJob(id string) (*Job, error) { func (jl *Solver) Get(id string) (*Job, error) { ctx, cancel := context.WithCancelCause(context.Background()) ctx, _ = context.WithTimeoutCause(ctx, 6*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() go func() { <-ctx.Done() diff --git a/solver/llbsolver/solver.go b/solver/llbsolver/solver.go index 659d63658..15a1f0911 100644 --- a/solver/llbsolver/solver.go +++ b/solver/llbsolver/solver.go @@ -204,7 +204,7 @@ func (s *Solver) recordBuildHistory(ctx context.Context, id string, req frontend ctx, cancel := context.WithCancelCause(ctx) ctx, _ = context.WithTimeoutCause(ctx, 300*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() var mu sync.Mutex ch := make(chan *client.SolveStatus) diff --git a/solver/scheduler.go b/solver/scheduler.go index cd94eea86..2011e5070 100644 --- a/solver/scheduler.go +++ b/solver/scheduler.go @@ -227,7 +227,7 @@ func (s *scheduler) build(ctx context.Context, edge Edge) (CachedResult, error) s.mu.Unlock() ctx, cancel := context.WithCancelCause(ctx) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() go func() { <-ctx.Done() diff --git a/solver/scheduler_test.go b/solver/scheduler_test.go index 897c2b92d..2d6f4c635 100644 --- a/solver/scheduler_test.go +++ b/solver/scheduler_test.go @@ -580,7 +580,7 @@ func TestSingleCancelParallel(t *testing.T) { }() ctx, cancel := context.WithCancelCause(ctx) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() g := Edge{ Vertex: vtx(vtxOpt{ diff --git a/source/containerimage/ocilayout.go b/source/containerimage/ocilayout.go index 345a5999b..ef579da03 100644 --- a/source/containerimage/ocilayout.go +++ b/source/containerimage/ocilayout.go @@ -126,7 +126,7 @@ func (r *ociLayoutResolver) withCaller(ctx context.Context, f func(context.Conte if r.store.SessionID != "" { timeoutCtx, cancel := context.WithCancelCause(ctx) timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() caller, err := r.sm.Get(timeoutCtx, r.store.SessionID, false) if err != nil { diff --git a/source/local/source.go b/source/local/source.go index 7c1711788..28a6f7aff 100644 --- a/source/local/source.go +++ b/source/local/source.go @@ -143,7 +143,7 @@ func (ls *localSourceHandler) Snapshot(ctx context.Context, g session.Group) (ca timeoutCtx, cancel := context.WithCancelCause(ctx) timeoutCtx, _ = context.WithTimeoutCause(timeoutCtx, 5*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() caller, err := ls.sm.Get(timeoutCtx, sessionID, false) if err != nil { diff --git a/util/flightcontrol/flightcontrol.go b/util/flightcontrol/flightcontrol.go index 59e5badb7..faf8411b2 100644 --- a/util/flightcontrol/flightcontrol.go +++ b/util/flightcontrol/flightcontrol.go @@ -118,7 +118,7 @@ func newCall[T any](fn func(ctx context.Context) (T, error)) *call[T] { func (c *call[T]) run() { defer c.closeProgressWriter(errors.WithStack(context.Canceled)) ctx, cancel := context.WithCancelCause(c.ctx) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() v, err := c.fn(ctx) c.mu.Lock() c.result = v @@ -157,7 +157,7 @@ func (c *call[T]) wait(ctx context.Context) (v T, err error) { } ctx, cancel := context.WithCancelCause(ctx) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() c.ctxs = append(c.ctxs, ctx) diff --git a/util/testutil/integration/registry.go b/util/testutil/integration/registry.go index 198069b65..0f2152de5 100644 --- a/util/testutil/integration/registry.go +++ b/util/testutil/integration/registry.go @@ -69,7 +69,7 @@ http: ctx, cancel := context.WithCancelCause(context.Background()) ctx, _ = context.WithTimeoutCause(ctx, 5*time.Second, errors.WithStack(context.DeadlineExceeded)) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() url, err = detectPort(ctx, rc) if err != nil { return "", nil, err diff --git a/util/testutil/integration/run.go b/util/testutil/integration/run.go index db5a3668c..1df2ccba2 100644 --- a/util/testutil/integration/run.go +++ b/util/testutil/integration/run.go @@ -198,7 +198,7 @@ func Run(t *testing.T, testCases []Test, opt ...TestOpt) { defer sandboxLimiter.Release(1) ctx, cancel := context.WithCancelCause(ctx) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() sb, closer, err := newSandbox(ctx, br, getMirror(), mv) require.NoError(t, err) diff --git a/util/tracing/otlptracegrpc/client.go b/util/tracing/otlptracegrpc/client.go index 5d54b1412..5e15772d5 100644 --- a/util/tracing/otlptracegrpc/client.go +++ b/util/tracing/otlptracegrpc/client.go @@ -70,7 +70,7 @@ func (c *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc } ctx, cancel := c.connection.ContextWithStop(ctx) - defer cancel(errors.WithStack(context.Canceled)) + defer func() { cancel(errors.WithStack(context.Canceled)) }() ctx, tCancel := context.WithCancelCause(ctx) ctx, _ = context.WithTimeoutCause(ctx, 30*time.Second, errors.WithStack(context.DeadlineExceeded)) defer tCancel(errors.WithStack(context.Canceled))