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.

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

cmd/vet: detect situations in which runtime.KeepAlive() must be added #16864

Closed
rasky opened this issue Aug 24, 2016 · 3 comments
Closed

cmd/vet: detect situations in which runtime.KeepAlive() must be added #16864

rasky opened this issue Aug 24, 2016 · 3 comments
Labels
FrozenDueToAge

Comments

@rasky
Copy link
Member

rasky commented Aug 24, 2016

There have been a couple of CLs in which @ianlancetaylor has manually added runtime.KeepAlive() after careful code inspection:

In Ian's words:

The cases that matter are where you have a parameter or local variable that is pointer to some type, there is or might be a finalizer on the object, the last reference to the object in the function is passing a field to some other function, and the finalizer might in some way invalidate that field value.

It would be nice if go vet could detect these situations, especially if it ends up being run on std regularly (eg. on trybots).

My proposal to make it possible is to add a tag vet:”finalizer_invalidate" to struct fields that matter in this context; with this tag, vet should be able to detect the missing calls to KeepAlive with no false positives.
@bradfitz
Copy link
Contributor

Nobody should use finalizers, so nobody should need this. Even if this sort of vet rule were easy to write (it won't be), I don't think we need to encourage the use of finalizers by making them easier.

@robpike
Copy link
Contributor

robpike commented Aug 24, 2016

I agree with Brad, plus it would be a bad precedent to introduce source-level markers to silence or enable vet warnings.

I vote against this proposal.

@rasky
Copy link
Member Author

rasky commented Aug 24, 2016

OK it was just a suggestion to help development of std. Never mind.

@rasky rasky closed this as completed Aug 24, 2016
@golang golang locked and limited conversation to collaborators Aug 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bradfitz @rasky @robpike @gopherbot