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: flag use of .Timeout() after Set*Deadline() #38508

Open
networkimprov opened this issue Apr 17, 2020 · 2 comments
Open

cmd/vet: flag use of .Timeout() after Set*Deadline() #38508

networkimprov opened this issue Apr 17, 2020 · 2 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Backlog

Comments

@networkimprov
Copy link

networkimprov commented Apr 17, 2020 •
edited

In #31449 it was decided to make deadline expiration return a newly-defined error, as checking net.OpError.Timeout() also detects keepalive failures. Keepalives were enabled by default for TCP clients in 1.12 and servers in 1.13, breaking some code setting deadlines longer than the keepalive period (which varies with platform).

This means that .Timeout() use after Set*Deadline() should be changed to look for the new error, since that is not a reliable way to check for deadline-expired.

I suggest go vet flag use of .Timeout() after Set*Deadline(). That will generate reports for existing code, but perhaps that's better than leaving code broken due to the keepalive problem.

Triage: milestone should be 1.15
@gopherbot add NeedsDecision
@gopherbot gopherbot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Apr 17, 2020
@networkimprov networkimprov mentioned this issue Apr 17, 2020
Closed
@ianlancetaylor
Copy link
Contributor

In order to make a decision here we need to see examples of existing code that should change. We also need to understand whether this vet check will have any false positives, and what such false positives look like in existing code.

@ianlancetaylor ianlancetaylor added this to the Go1.15 milestone Apr 17, 2020
@jayconrod jayconrod modified the milestones: Go1.15, Backlog Apr 27, 2020
@jayconrod
Copy link
Contributor

Moving to Backlog milestone. There are a lot of open 1.15 issues, and unless someone is committing to working on this, it doesn't seem likely to happen in that timeframe.

@adonovan adonovan added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@jayconrod @networkimprov @ianlancetaylor @adonovan @gopherbot