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 97db913bad76..95084276374a 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 f8fb1df27ae4..10e746109432 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 c801eea31274..ab2e3d1ccfec 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 bae7210792eb..48a05e494fe3 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 523c605104f2..7bfd8f7d94c3 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 3012c1e269a1..5b51d252f348 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 49e12c285964..f1a8eab8364e 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 a5228cb330e1..225503b3d52d 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 93fe9265fb2a..df8e4134eaa5 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 1b6f36f774bb..96eedffe8c3d 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 d8f458192c79..e777690d578d 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 4bdb25f02687..4c7e4b233739 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 e701d1d29206..66d18fcc7a8e 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 02383dc090eb..f411f0a3ba63 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 bb2bccccb03c..b67e25ecee7b 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 5b1e121989a2..d8cce3abdcf7 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 659d636582e3..15a1f0911372 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 cd94eea86eaa..2011e5070376 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 897c2b92d5af..2d6f4c6357f9 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 345a5999b30c..ef579da03a84 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 7c17117886ce..28a6f7affd4f 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 59e5badb7a78..faf8411b235f 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 198069b65992..0f2152de5383 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 db5a3668c108..1df2ccba201d 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 5d54b1412944..5e15772d5117 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))