Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

time: the finalizers of Timer objects will not get executed #66650

Open
go101 opened this issue Apr 2, 2024 · 5 comments
Open

time: the finalizers of Timer objects will not get executed #66650

go101 opened this issue Apr 2, 2024 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Go1.23

Comments

@go101
Copy link

go101 commented Apr 2, 2024

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
$
@go101 go101 mentioned this issue Apr 2, 2024
Closed
@ianlancetaylor
Copy link
Contributor

CC @rsc @golang/runtime

@ianlancetaylor ianlancetaylor added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 2, 2024
@aclements
Copy link
Member

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.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 2, 2024
@mknyszek mknyszek added this to the Backlog milestone Apr 10, 2024
@mknyszek
Copy link
Contributor

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.

@mknyszek mknyszek added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 10, 2024
@mknyszek mknyszek modified the milestones: Backlog, Go1.23 Apr 10, 2024
@ianlancetaylor
Copy link
Contributor

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
Copy link
Contributor

mknyszek commented Apr 11, 2024 •
edited

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
Go Compiler / Runtime
Status: Todo
Milestone
Go1.23
Development

No branches or pull requests
5 participants
@mknyszek @dmitshur @aclements @ianlancetaylor @go101