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
net: Conn hangs with SetReadDeadline #34385
Comments
This commits fixes a bug introduced in af6c6a2. This workaround is needed to fix the upstream Go issue golang/go#34385 related to Deadlines Fixes minio#7852
The program you showed us doesn't look at |
@ianlancetaylor my bad here is the right example
|
Had to update the example, updated the right link. |
This commits fixes a bug introduced in af6c6a2. Setting deadlines in Go results in arbitrary hangs as reported here golang/go#34385 Fixes minio#7852
This commits fixes a bug introduced in af6c6a2. Setting deadlines in Go results in arbitrary hangs as reported here golang/go#34385 Fixes minio#7852
This commits fixes a bug introduced in af6c6a2. Setting deadlines in Go results in arbitrary hangs as reported here golang/go#34385 Fixes minio#7852
If |
@av86743 I don't think so it does here is how I tested the shared code. package main
import (
"log"
"net"
"net/http"
"os"
"time"
)
// timeoutableConn
type timeoutableConn struct {
readTimeout time.Duration
net.Conn
}
func (c *timeoutableConn) Read(b []byte) (n int, err error) {
if c.readTimeout > 0 {
//c.SetReadDeadline(time.Now().Add(c.readTimeout))
}
n, err = c.Conn.Read(b)
return n, err
}
// customListener returns timeoutable net.Conn
type customListener struct {
net.Listener
}
func (l *customListener) Accept() (conn net.Conn, err error) {
c, e := l.Listener.Accept()
if e != nil {
return nil, e
}
return &timeoutableConn{Conn: c, readTimeout: 5 * time.Second}, nil
}
func main() {
var l net.Listener
l, err := net.Listen("tcp", ":1234")
if err != nil {
log.Fatal(err)
}
if os.Getenv("NO_TIMEOUT") != "1" {
l = &customListener{l}
}
_ = http.Serve(l, http.HandlerFunc(
func(w http.ResponseWriter, req *http.Request) {
buf := []byte(`127.0.0.1 localhost
127.0.1.1 backspace
# The following lines are desirable for IPv6 capable hosts
::1 ip6-localhost ip6-loopback
fe00::0 ip6-localnet
ff00::0 ip6-mcastprefix
ff02::1 ip6-allnodes
ff02::2 ip6-allrouters
172.31.59.24 ceph-1
172.31.57.119 ceph-2
172.31.57.137 ceph-3
172.31.63.24 ceph-4
172.31.63.204 ceph-5
172.31.51.188 ceph-6
172.31.54.140 ceph-7
172.31.49.145 ceph-8
`)
w.Write(buf)
w.(http.Flusher).Flush()
}))
}
So no hangs if that code is commented out. Add the code back.
So there is no issue in the code itself, I am not sure how are you testing it. |
Run on playground. |
Playground doesn't run HTTP service @av86743 its a nacl container :-) any long-running go-routines such as HTTP service are rejected. That is not how you would reproduce this issue IMHO, run it locally on your laptop. |
First: Why put your example in playground if it is not runnable from there? Second: An original example from #21133 with your setting of port Third: #21133 was closed without any intention to fix anything in net/http. Why do you want to present it differently as:
|
This commit fixes a bug introduced in af6c6a2. Setting deadlines in Go results in arbitrary hangs as reported here golang/go#34385 Fixes #7852
@av86743 because original was never addressed, what I wanted to shed light on is that the workaround provided was also not correct because. It basically disabled SetReadDeadline() altogether and that exactly what shouldn't happen. Setting deadlines repeatedly shouldn't cause hangs in the first place and the workaround is also wrong because it basically just disabled the SetReadDeadline() code.
This is an odd argument, the intention is to show code how to reproduce it. This issue also talks about how to reproduce it using the command line using PHP code. I don't know what made you think that you can run in the playground and also run PHP code to reproduce this.
The original example added client code, which I didn't want to add because you need to use PHP to reproduce this issue not Go HTTP client. Please follow what I pasted above on how to reproduce it. |
This is not a question and as such, it does not require your response. This is statement of a fact contrary to what you have earlier stated (I quote):
If you are not able to reproduce whatever it is without involving PHP, why do you claim that problem is on the go side and not on the PHP side? |
@harshavardhana It's documented in #21133 why your code doesn't work, and what the workaround is. If the workaround has problems, you should open a new issue with code that demonstrates them. Note curl is a better test client; not that many folks have PHP installed. P.S. you don't have to respond to every comment you get on an issue :-) |
Right now its just PHP - I haven't explored enough to show you if there are other clients out there which reproduce this issue - the reason is that without deadlines the code works fine. That makes me wonder that the issue is in Go - either deadlines don't work the way we want or we are using it wrong. Some explanation that would help in the documentation.
Fundamental reasoning is why does one need a workaround in the first place, the issue is closed as if the bug is in our custom net.Conn implementation which doesn't seem to be the case unless our usage of deadlines are dead wrong - if that is the case I am happy to change the implementation instead of applying a workaround. So any guidance on that area would be much appreciated. |
EDIT: Please re-read #21133 (comment). |
When read deadline is not set in |
Okay, understood @av86743 - with that analogy it would certainly also mean that writing custom wrappers for net.Conn with deadlines is generally not advisable and can lead to issues - also come in the way. Would it be possible to document such a thing? Because we shot ourselves in the foot without realizing this would come in the way as per net.Conn documentation. |
Is there an intent to document this on why writing custom net.Conn's with deadlines won't work in reality with net/http? |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
https://play.golang.org/p/Xpw5O5EsXQvhttps://play.golang.org/p/C-Jdtg6Wr9M
What did you expect to see?
No hangs with following code
What did you see instead?
Hangs up to the SetReadDeadline timeout
This is another variant of the same issue #21133 and the work-around presented works still.
The question I have is why this workaround is needed at all. Is it not expected to SetReadDeadline() right before the read operation? but why does it work some times? It also looks like there may be some performance implications on doing something like this.
The text was updated successfully, but these errors were encountered: