Skip to content

Commit

Permalink
runtime: add mayAcquire annotation for finlock
Browse files Browse the repository at this point in the history
We're missing lock edges to finlock that happen only rarely. Anything
that calls mallocgc can potentially trigger sweeping, which can
potentially queue a finalizer, which acquires finlock. While this can
happen on any malloc, it happens relatively rarely, so we simply
haven't seen some of the lock edges that could happen.

Add a mayAcquire annotation to mallocgc to capture the possibility of
acquiring finlock.

With this change, we add "fin" to the set of "malloc" locks. Several
of these edges were already there, but not quite all of them.

This was found by inspecting the rank graph for things that didn't
make sense.

For #53789.

Change-Id: Idc10ce6f250596b0c07ba07ac93f2198fb38c22b
Reviewed-on: https://go-review.googlesource.com/c/go/+/418717
Run-TryBot: Austin Clements <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
  • Loading branch information
aclements committed Aug 4, 2022
1 parent c5be4ed commit d29a028
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 14 deletions.
24 changes: 12 additions & 12 deletions src/runtime/lockrank.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/runtime/malloc.go
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,11 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer {
if size == 0 {
return unsafe.Pointer(&zerobase)
}

// It's possible for any malloc to trigger sweeping, which may in
// turn queue finalizers. Record this dynamic lock edge.
lockRankMayQueueFinalizer()

userSize := size
if asanenabled {
// Refer to ASAN runtime library, the malloc() function allocates extra memory,
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/mfinal.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ var finalizer1 = [...]byte{
0<<0 | 1<<1 | 1<<2 | 1<<3 | 1<<4 | 0<<5 | 1<<6 | 1<<7,
}

// lockRankMayQueueFinalizer records the lock ranking effects of a
// function that may call queuefinalizer.
func lockRankMayQueueFinalizer() {
lockWithRankMayAcquire(&finlock, getLockRank(&finlock))
}

func queuefinalizer(p unsafe.Pointer, fn *funcval, nret uintptr, fint *_type, ot *ptrtype) {
if gcphase != _GCoff {
// Currently we assume that the finalizer queue won't
Expand Down
3 changes: 1 addition & 2 deletions src/runtime/mklockrank.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ allp < timers;
itab < reflectOffs;
scavenge, sweep < hchan;
scavenge < traceBuf;
allg, hchan, reflectOffs, timers, traceBuf < fin;
traceBuf < traceStrings;
allg, hchan, notifyList, reflectOffs, timers, traceStrings < mspanSpecial, profInsert, profBlock, profMemActive, gcBitsArenas, spanSetSpine, mheapSpecial;
allg, hchan, notifyList, reflectOffs, timers, traceStrings < mspanSpecial, profInsert, profBlock, profMemActive, gcBitsArenas, spanSetSpine, mheapSpecial, fin;
profMemActive < profMemFuture;
hchan, root, sched, traceStrings, notifyList, fin < trace;
trace < traceStackTab;
Expand Down

0 comments on commit d29a028

Please sign in to comment.