From 39b7178450a73f7e64d98816a30228974894b04e Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Mon, 18 Jul 2022 16:35:11 -0700 Subject: [PATCH] sql: prevent span use after finish with rollbacks and high vmodule Previously, we could get a panic of "span use after finish" in test builds in some cases (like a rollback). This was occurring since the context cancellation function can log some stuff depending on the verbosity level (in `contextutil/context.go:74`) long time after we have finished the tracing span from that context. To go around that issue we now push the responsibility of finishing the span into the cancellation function. Release note: None --- pkg/sql/exec_util.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/sql/exec_util.go b/pkg/sql/exec_util.go index 56032b33d0dd..e68e144a486a 100644 --- a/pkg/sql/exec_util.go +++ b/pkg/sql/exec_util.go @@ -2285,9 +2285,17 @@ func (st *SessionTracing) StartTracing( if sp == nil { return errors.Errorf("no txn span for SessionTracing") } - // We're hijacking this span and we're never going to un-hijack it, so it's - // up to us to finish it. - sp.Finish() + // Since we're hijacking this span and we're never going to un-hijack + // it, it's up to us to make sure it is finished somehow. Unfortunately, + // we cannot just finish it right away because in some cases the span + // will be used later (if the verbosity is high enough, during the + // context cancellation). To go around that, we pass the responsibility + // of finishing the span to the cancellation function. + cancelTxnCtx := st.ex.state.cancel + st.ex.state.cancel = func() { + cancelTxnCtx() + sp.Finish() + } st.ex.state.Ctx, _ = tracing.EnsureChildSpan( newConnCtx, st.ex.server.cfg.AmbientCtx.Tracer, "session tracing")