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

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

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

Comments

Projects
None yet
4 participants
@rasky
Copy link
Member

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

This comment has been minimized.

Copy link
Member

commented Aug 24, 2016

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2016

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

@rasky rasky closed this 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.
You can’t perform that action at this time.