Skip to content

Commit

Permalink
util/tracing: disable span reuse under deadlock detector
Browse files Browse the repository at this point in the history
In cockroachdb#73883 we started reusing tracing spans through a sync.Pool. This
causes the mutexes in the spans to be acquired by a goroutine
repeatedly, in a random order. Multiple spans can be locked at the same
time, but the ordering well defined (a child can be locked while the
parent is also locked). This erroneously looks like a possible deadlock
to the deadlock detector. The following test demonstrates the problem -
it's flagged by the deadlock detector even though the code is
deadlock-free.

This patch fixes the issue by disabling span reuse under deadlock
builds.

I guess the moral of the story is that recycling structs with locks
through sync.Pool is incompatible with the deadlock detector? One
option, I guess, is to contribute a way to mark some mutexes as excluded
from the deadlock detector.

func TestXXX(t *testing.T) {
	p := sync.Pool{New: func() interface{} {
		return new(syncutil.Mutex)
	},
	}

	var m1, m2 *syncutil.Mutex
	m1 = p.Get().(*syncutil.Mutex)
	m2 = p.Get().(*syncutil.Mutex)

	m1.Lock()
	m2.Lock()
	m2.Unlock()
	m1.Unlock()

	p.Put(m1)
	p.Put(m2)
	m2 = p.Get().(*syncutil.Mutex)
	m1 = p.Get().(*syncutil.Mutex)

	m1.Lock()
	m2.Lock()
	m2.Unlock()
	m1.Unlock()
}

Fixes cockroachdb#75351 and a million others.

Release note: None
  • Loading branch information
andreimatei authored and Gerardo Torres committed Jan 24, 2022
1 parent b0a8752 commit 885409d
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion pkg/util/tracing/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,12 @@ var debugUseAfterFinish = envutil.EnvOrDefaultBool("COCKROACH_DEBUG_SPAN_USE_AFT

// reuseSpans controls whether spans can be re-allocated after they've been
// Finish()ed. See Tracer.spanReusePercent for details.
var reuseSpans = buildutil.CrdbTestBuild ||
//
// Span reuse is incompatible with the deadlock detector because, with reuse,
// the span mutexes end up being reused and locked repeatedly in random order on
// the same goroutine. This erroneously looks like a potential deadlock to the
// detector.
var reuseSpans = (buildutil.CrdbTestBuild && !syncutil.DeadlockEnabled) ||
envutil.EnvOrDefaultBool("COCKROACH_REUSE_TRACING_SPANS", false)

// detectSpanRefLeaks enables the detection of Span reference leaks - i.e.
Expand Down

0 comments on commit 885409d

Please sign in to comment.