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: 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
Milestone

Comments

@networkimprov
Copy link

@networkimprov networkimprov commented Apr 17, 2020

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

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 17, 2020

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

@jayconrod jayconrod commented Apr 27, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.