Skip to content

net: fix Dialer, DialTimeout regression #11796

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

Closed
dsymonds opened this issue Jul 20, 2015 · 5 comments
Closed

net: fix Dialer, DialTimeout regression #11796

dsymonds opened this issue Jul 20, 2015 · 5 comments
Milestone

Comments

@dsymonds
Copy link
Contributor

Since 0d8366e, net.DialTimeout appears to no longer respect timeouts shorter than 2s. That is, if I do a net.DialTimeout("tcp", "some-addr", 500*time.Millisecond) where "some-addr" is something that resolves quickly (with both IPv4 and IPv6 addresses) but doesn't respond to connections, net.DialTimeout takes a little over 1s to return.

This has changed since Go 1.4.2. I don't know whether this was an intentional change, or a regression; the former change warrants a mention in the release notes.

@pmarks-net @bradfitz @mikioh

@dsymonds dsymonds added this to the Go1.5 milestone Jul 20, 2015
dsymonds added a commit to golang/appengine that referenced this issue Jul 20, 2015
golang/go#11796 has details of the bug.

Change-Id: I94aa78685c962c32652b227b1597076c305977ac
@pmarks-net
Copy link
Contributor

Could you clarify what problem you're seeing? The 2s saneMinimum only affects how time is distributed across multiple addresses; it should not change the overall deadline. The following example shows a short timeout working as expected:

$ cat ~/test/dial_fail.go
package main

import (
        "net"
        "fmt"
        "time"
)

func main() {
        startTime := time.Now()
        conn, err := net.DialTimeout("tcp", "192.0.2.1:80", 123*time.Millisecond)
        fmt.Printf("%v %v\n", conn, err)
        fmt.Printf("Finished in %v\n", time.Now().Sub(startTime))
}

$ ../bin/go run ~/test/dial_fail.go
<nil> dial tcp 192.0.2.1:80: i/o timeout
Finished in 123.238884ms

@dsymonds
Copy link
Contributor Author

dsymonds commented Jul 20, 2015 via email

@pmarks-net
Copy link
Contributor

Ah, I am able to reproduce a bug in the partialDeadline calculation. When dialing 2 addresses with a 5 second timeout, the target deadline is recomputed for each address, instead of once at the beginning of Dial:

$ ../bin/go run ~/test/dial_fail.go 
partialDeadline=2.499999662s
partialDeadline=4.999999028s
<nil> dial tcp [2001:db8::1]:80: i/o timeout
Finished in 7.500665372s

I introduced the bug between patch sets 3-4 of the code review:
https://go-review.googlesource.com/#/c/8768/3..4/src/net/dial.go

@pmarks-net
Copy link
Contributor

There is actually a second, pre-existing bug: DialTimeout(..., 5s) will allocate 5 seconds to DNS resolution, and another 5 seconds to TCP.

The fix for both is to call d.deadline(...) exactly once, at the beginning of Dial.

@bradfitz bradfitz changed the title Document or fix net.DialTimeout regression net: document or fix DialTimeout regression Jul 20, 2015
@gopherbot
Copy link
Contributor

CL https://golang.org/cl/12442 mentions this issue.

@dsymonds dsymonds removed the Started label Jul 23, 2015
@mikioh mikioh changed the title net: document or fix DialTimeout regression net: fix Dialer, DialTimeout regression Jul 23, 2015
@golang golang locked and limited conversation to collaborators Aug 5, 2016
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