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

x/net/nettest: Conn.SetDeadline doc comment and test comment conflict #30275

Closed
santhosh-tekuri opened this issue Feb 17, 2019 · 6 comments
Closed

Comments

@santhosh-tekuri
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.10.3 windows/amd64

Does this issue reproduce with the latest release?

this is related to documentation

net.Conn.setDeadline says:

The deadline applies to all future and pending I/O, 
not just the immediately following call to ReadFrom 
or WriteTo

This means I can extend previously set deadline to longer period.

but when i look at nettest.testPresentTimeout it says:

testPresentTimeout tests that a deadline set while there are pending
Read and Write operations immediately times out those operations.

if so, net.Conn.SetDeadline should say that:
changing timeout will cause any pending I/O calls to timeout

By looking at nettest.testPresentTimeout code, I see that it is not extending current deadline. so
test is correct but its doc is wrong. I tried to see if there is any test, which tests extending deadline of
pending IO, but could not find

for clarification, i checked net.Pipe implementation:

any pending net.pipe.Read will always timeout on setDeadline
any pending net.pipe.Write may sometimes timeout on setDeadline and sometime not (if setDeadline called between for loop iterations

@mikioh
Copy link
Contributor

mikioh commented Feb 18, 2019

@santhosh-tekuri,

Can you please confirm that the docs you are suggesting are really for the net.Conn interface instead of net.PacketConn interface.

@mikioh mikioh added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Feb 18, 2019
@santhosh-tekuri
Copy link
Contributor Author

what I am suggesting is to correct doc of nettest.testPresentTimeout.
I know it is not part of public api, But who is digging into std lib, this doc comment can confuse

@santhosh-tekuri
Copy link
Contributor Author

the doc of nettest.testPresentTimeout should say something like:

testPresentTimeout tests that a  past deadline set while there are pending
Read and Write operations immediately times out those operations.

@mikioh mikioh changed the title net: Conn.SetDeadline doc comment and test comment conflict x/net/nettest: Conn.SetDeadline doc comment and test comment conflict Feb 18, 2019
@gopherbot gopherbot added this to the Unreleased milestone Feb 18, 2019
@mikioh mikioh removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Feb 18, 2019
@mikioh
Copy link
Contributor

mikioh commented Feb 18, 2019

Then, feel free to send a CL; the target package is https://godoc.org/github.com/golang/net/nettest.

@santhosh-tekuri
Copy link
Contributor Author

will send CL

@gopherbot
Copy link

Change https://golang.org/cl/162923 mentions this issue: x/net/nettest: fix doc for testPresentTimeout

@golang golang locked and limited conversation to collaborators Feb 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants