qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-26 05:17:58 -07:00
Code Releases Activity

runtime: don't hold the table lock in (*traceStackTable).dump

Browse Source 
There's a conceptual cycle between traceStackTable.lock and
allocation-related locks, but it can't happen in practice because the
caller guarantees that there are no more writers to the table at the
point that dump is called.

But if that's true, then the lock isn't necessary at all. It would be
difficult to model this quiesence in the lockrank mode, so just don't
hold the lock and expand the documentation of the dump method.

Change-Id: Id4db61363f075b7574135529915e8bd4f4f4c082
Reviewed-on: https://go-review.googlesource.com/c/go/+/544177
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This commit is contained in:
Michael Anthony Knyszek 2023-11-21 22:42:35 +00:00 committed by Michael Knyszek
parent d4430f6913
commit 5249947f11
1 changed files with 14 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/runtime/trace2stack.go
View File

@ -97,7 +97,8 @@ func (t *traceStackTable) put(pcs []uintptr) uint64 {
}
// dump writes all previously cached stacks to trace buffers,
// releases all memory and resets state.
// releases all memory and resets state. It must only be called once the caller
// can guarantee that there are no more writers to the table.
//
// This must run on the system stack because it flushes buffers and thus
// may acquire trace.lock.
@ -107,7 +108,15 @@ func (t *traceStackTable) dump(gen uintptr) {
w := unsafeTraceWriter(gen, nil)
// Iterate over the table.
lock(&t.tab.lock)
//
// Do not acquire t.tab.lock. There's a conceptual lock cycle between acquiring this lock
// here and allocation-related locks. Specifically, this lock may be acquired when an event
// is emitted in allocation paths. Simultaneously, we might allocate here with the lock held,
// creating a cycle. In practice, this cycle is never exercised. Because the table is only
// dumped once there are no more writers, it's not possible for the cycle to occur. However
// the lockrank mode is not sophisticated enough to identify this, and if it's not possible
// for that cycle to happen, then it's also not possible for this to race with writers to
// the table.
for i := range t.tab.tab {
stk := t.tab.bucket(i)
for ; stk != nil; stk = stk.next() {
@ -144,6 +153,9 @@ func (t *traceStackTable) dump(gen uintptr) {
}
}
}
// Still, hold the lock over reset. The callee expects it, even though it's
// not strictly necessary.
lock(&t.tab.lock)
t.tab.reset()
unlock(&t.tab.lock)