Skip to content

Commit 9a5b055

Browse files
committed
runtime: update docs, code for SetFinalizer
At last minute before 1.3 we relaxed SetFinalizer to avoid crashes when you pass the result of a global alloc to it. This avoids the crash but makes SetFinalizer a bit too relaxed. Document that the finalizer of a global allocation may not run. Tighten the SetFinalizer check to ignore a global allocation but not ignore everything else. Fixes #7656. LGTM=r, iant R=golang-codereviews, iant, r CC=dvyukov, golang-codereviews, khr, rlh https://golang.org/cl/145930043
1 parent 609d996 commit 9a5b055

File tree

2 files changed

+33
-17
lines changed

2 files changed

+33
-17
lines changed

src/runtime/malloc.go

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,10 @@ func GC() {
488488
gogc(2)
489489
}
490490

491+
// linker-provided
492+
var noptrdata struct{}
493+
var enoptrbss struct{}
494+
491495
// SetFinalizer sets the finalizer associated with x to f.
492496
// When the garbage collector finds an unreachable block
493497
// with an associated finalizer, it clears the association and runs
@@ -527,6 +531,10 @@ func GC() {
527531
// It is not guaranteed that a finalizer will run if the size of *x is
528532
// zero bytes.
529533
//
534+
// It is not guaranteed that a finalizer will run for objects allocated
535+
// in initializers for package-level variables. Such objects may be
536+
// linker-allocated, not heap-allocated.
537+
//
530538
// A single goroutine runs all finalizers for a program, sequentially.
531539
// If a finalizer must run for a long time, it should do so by starting
532540
// a new goroutine.
@@ -544,24 +552,25 @@ func SetFinalizer(obj interface{}, finalizer interface{}) {
544552
gothrow("nil elem type!")
545553
}
546554

547-
// As an implementation detail we do not run finalizers for zero-sized objects,
548-
// because we use &runtime·zerobase for all such allocations.
549-
if ot.elem.size == 0 {
550-
return
551-
}
552-
553555
// find the containing object
554556
_, base, _ := findObject(e.data)
555557

556-
// The following check is required for cases when a user passes a pointer to composite
557-
// literal, but compiler makes it a pointer to global. For example:
558-
// var Foo = &Object{}
559-
// func main() {
560-
// runtime.SetFinalizer(Foo, nil)
561-
// }
562-
// See issue 7656.
563558
if base == nil {
564-
return
559+
// 0-length objects are okay.
560+
if e.data == unsafe.Pointer(&zerobase) {
561+
return
562+
}
563+
564+
// Global initializers might be linker-allocated.
565+
// var Foo = &Object{}
566+
// func main() {
567+
// runtime.SetFinalizer(Foo, nil)
568+
// }
569+
// The segments are, in order: text, rodata, noptrdata, data, bss, noptrbss.
570+
if uintptr(unsafe.Pointer(&noptrdata)) <= uintptr(e.data) && uintptr(e.data) < uintptr(unsafe.Pointer(&enoptrbss)) {
571+
return
572+
}
573+
gothrow("runtime.SetFinalizer: pointer not in allocated block")
565574
}
566575

567576
if e.data != base {

src/runtime/mfinal_test.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,17 @@ func TestFinalizerType(t *testing.T) {
4444
{func(x *int) interface{} { return (*Tint)(x) }, func(v Tinter) { finalize((*int)(v.(*Tint))) }},
4545
}
4646

47-
for _, tt := range finalizerTests {
47+
for i, tt := range finalizerTests {
4848
done := make(chan bool, 1)
4949
go func() {
50-
v := new(int)
50+
// allocate struct with pointer to avoid hitting tinyalloc.
51+
// Otherwise we can't be sure when the allocation will
52+
// be freed.
53+
type T struct {
54+
v int
55+
p unsafe.Pointer
56+
}
57+
v := &new(T).v
5158
*v = 97531
5259
runtime.SetFinalizer(tt.convert(v), tt.finalizer)
5360
v = nil
@@ -58,7 +65,7 @@ func TestFinalizerType(t *testing.T) {
5865
select {
5966
case <-ch:
6067
case <-time.After(time.Second * 4):
61-
t.Errorf("finalizer for type %T didn't run", tt.finalizer)
68+
t.Errorf("#%d: finalizer for type %T didn't run", i, tt.finalizer)
6269
}
6370
}
6471
}

0 commit comments

Comments
 (0)