time: the finalizers of Timer objects will not get executed

[zigo101]

Go version

go version devel go1.23-9a028e14a5 Fri Mar 29 16:46:47 2024 +0000 linux/amd64

Output of go env in your module/workspace:

.

What did you do?

Moved from #37196 (comment)

package main

import "time"
import "runtime"

func main() {
	timer := time.NewTimer(time.Second)
	runtime.SetFinalizer(timer, func(c *time.Timer){
		println("done")
	})
	timer.Stop()
	runtime.GC()
	time.Sleep(time.Second)
}

What did you see happen?

$ gotv 1.22. run main.go
[Run]: $HOME/.cache/gotv/tag_go1.22.1/bin/go run main.go
done
$ gotv :tip run main.go
[Run]: $HOME/.cache/gotv/bra_master/bin/go run main.go
$ 

What did you expect to see?

$ gotv 1.22. run main.go
[Run]: $HOME/.cache/gotv/tag_go1.22.1/bin/go run main.go
done
$ gotv :tip run main.go
[Run]: $HOME/.cache/gotv/bra_master/bin/go run main.go
done
$ 

[ianlancetaylor]

CC @rsc @golang/runtime

[aclements]

I haven't dug into this, but one observation: In general, timers are deleted lazily from the heap (for concurrency reasons). This program is doing little enough that it's possible it was just getting lucky before and tickling a timer cleanup, and now it's not getting quite as lucky and it's not triggering a timer cleanup.

[mknyszek]

I poked at this and I think there might be a real leak here, but only if you call SetFinalizer on a timer. Each runtime.timer has a reference to a runtime.timers which in turn references back to the runtime.timer. That is, there's a reference cycle here. Self-reference cycles with finalizers on the cycle cannot be collected because the finalizer-having-object's referents must be treated as roots (thanks to object resurrection).

I am uncertain if this is worth fixing.

[ianlancetaylor]

It's not obvious to me why that is a problem. The timer.ts field, which points to a timers, should always be nil when the timer is not referenced by any timers. And if the timer is reference by a timers, then it can't be collected and we can't run the finalizer. So as far as I can see there shouldn't be an actual reference loop if the timer can be collected.

[mknyszek]

My bad. This is a timer channel anyway, so it's not heaped at all.

However, I see that it's possible for an hchan to have a reference to the timer, while the timer has a reference to the hchan, so I believe there's still a reference loop here. (The timer references the channel via arg AFAICT.) I do not see that reference loop getting cleared anywhere.

Again, not sure if this is worth fixing. It might be as simple as just clearing the arg and f fields from stop, but I'm not familiar enough with the implementation to say for sure.