Commit d3836aba authored by Austin Clements's avatar Austin Clements

runtime: ensure finalizers are zero-initialized before reuse

We reuse finalizers in finblocks, which are allocated off-heap. This
means they have to be zero-initialized before becoming visible to the
garbage collector. We actually already do this by clearing the
finalizer before returning it to the pool, but we're not careful to
enforce correct memory ordering. Fix this by manipulating the
finalizer count atomically so these writes synchronize properly with
the garbage collector.

Updates #17503.

Change-Id: I7797d31df3c656c9fe654bc6da287f66a9e2037d
Reviewed-on: https://go-review.googlesource.com/31454Reviewed-by: default avatarRick Hudson <rlh@golang.org>
parent db56a635
...@@ -19,7 +19,7 @@ import ( ...@@ -19,7 +19,7 @@ import (
type finblock struct { type finblock struct {
alllink *finblock alllink *finblock
next *finblock next *finblock
cnt int32 cnt uint32
_ int32 _ int32
fin [(_FinBlockSize - 2*sys.PtrSize - 2*4) / unsafe.Sizeof(finalizer{})]finalizer fin [(_FinBlockSize - 2*sys.PtrSize - 2*4) / unsafe.Sizeof(finalizer{})]finalizer
} }
...@@ -72,7 +72,7 @@ var finalizer1 = [...]byte{ ...@@ -72,7 +72,7 @@ var finalizer1 = [...]byte{
func queuefinalizer(p unsafe.Pointer, fn *funcval, nret uintptr, fint *_type, ot *ptrtype) { func queuefinalizer(p unsafe.Pointer, fn *funcval, nret uintptr, fint *_type, ot *ptrtype) {
lock(&finlock) lock(&finlock)
if finq == nil || finq.cnt == int32(len(finq.fin)) { if finq == nil || finq.cnt == uint32(len(finq.fin)) {
if finc == nil { if finc == nil {
finc = (*finblock)(persistentalloc(_FinBlockSize, 0, &memstats.gc_sys)) finc = (*finblock)(persistentalloc(_FinBlockSize, 0, &memstats.gc_sys))
finc.alllink = allfin finc.alllink = allfin
...@@ -99,7 +99,7 @@ func queuefinalizer(p unsafe.Pointer, fn *funcval, nret uintptr, fint *_type, ot ...@@ -99,7 +99,7 @@ func queuefinalizer(p unsafe.Pointer, fn *funcval, nret uintptr, fint *_type, ot
finq = block finq = block
} }
f := &finq.fin[finq.cnt] f := &finq.fin[finq.cnt]
finq.cnt++ atomic.Xadd(&finq.cnt, +1) // Sync with markroots
f.fn = fn f.fn = fn
f.nret = nret f.nret = nret
f.fint = fint f.fint = fint
...@@ -112,7 +112,7 @@ func queuefinalizer(p unsafe.Pointer, fn *funcval, nret uintptr, fint *_type, ot ...@@ -112,7 +112,7 @@ func queuefinalizer(p unsafe.Pointer, fn *funcval, nret uintptr, fint *_type, ot
//go:nowritebarrier //go:nowritebarrier
func iterate_finq(callback func(*funcval, unsafe.Pointer, uintptr, *_type, *ptrtype)) { func iterate_finq(callback func(*funcval, unsafe.Pointer, uintptr, *_type, *ptrtype)) {
for fb := allfin; fb != nil; fb = fb.alllink { for fb := allfin; fb != nil; fb = fb.alllink {
for i := int32(0); i < fb.cnt; i++ { for i := uint32(0); i < fb.cnt; i++ {
f := &fb.fin[i] f := &fb.fin[i]
callback(f.fn, f.arg, f.nret, f.fint, f.ot) callback(f.fn, f.arg, f.nret, f.fint, f.ot)
} }
...@@ -208,11 +208,14 @@ func runfinq() { ...@@ -208,11 +208,14 @@ func runfinq() {
reflectcall(nil, unsafe.Pointer(f.fn), frame, uint32(framesz), uint32(framesz)) reflectcall(nil, unsafe.Pointer(f.fn), frame, uint32(framesz), uint32(framesz))
fingRunning = false fingRunning = false
// drop finalizer queue references to finalized object // Drop finalizer queue heap references
// before hiding them from markroot.
// This also ensures these will be
// clear if we reuse the finalizer.
f.fn = nil f.fn = nil
f.arg = nil f.arg = nil
f.ot = nil f.ot = nil
fb.cnt = i - 1 atomic.Store(&fb.cnt, i-1)
} }
next := fb.next next := fb.next
lock(&finlock) lock(&finlock)
......
...@@ -186,7 +186,8 @@ func markroot(gcw *gcWork, i uint32) { ...@@ -186,7 +186,8 @@ func markroot(gcw *gcWork, i uint32) {
case i == fixedRootFinalizers: case i == fixedRootFinalizers:
for fb := allfin; fb != nil; fb = fb.alllink { for fb := allfin; fb != nil; fb = fb.alllink {
scanblock(uintptr(unsafe.Pointer(&fb.fin[0])), uintptr(fb.cnt)*unsafe.Sizeof(fb.fin[0]), &finptrmask[0], gcw) cnt := uintptr(atomic.Load(&fb.cnt))
scanblock(uintptr(unsafe.Pointer(&fb.fin[0])), cnt*unsafe.Sizeof(fb.fin[0]), &finptrmask[0], gcw)
} }
case i == fixedRootFreeGStacks: case i == fixedRootFreeGStacks:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment