-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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/http: TestTransportRemovesDeadIdleConnections failing on Plan 9 #15464
Comments
I've added some debug prints:
The server listens on /net/tcp/11. The client opens a connection on /net/tcp/12 which is accepted by the server on /net/tcp/18. In readLoop, on the first iteration (numExpectedResponses=1), the client calls Peek(1) and reads 132 bytes. Then, the server closes the connection on his side. On the second iteration (numExpectedResponses=0), the client calls Peek(1), but the Read never returns. Consenquently, we never reach the "numExpectedResponses == 0" test to leave readLoop. This happens because server closes the connection on his side (/net/tcp/18), but the connection is still open on client's side (/net/tcp/12). This issue can be reproduced independently of Go, like this: Listen on port 8080:
Connect to port 8080:
Figure out what is the connection file on the server side:
Figure out what is the connection file on the client side:
Close the connection from the server side:
The connection on the client side is still on "established" state:
Closing the connection with "hangup" is supposed to send a RST to the other side. I'm not sure why the connection remains in "established" state. I'll investigate. FYI @bradfitz |
I noticed a mistake in my previous test. This is the listener file:
This is the connection file on the server side:
Close the connection from the server side:
Closing the connection from the server side sends RST to the client, which closes the connection on his side, as expected:
I'm still not sure why it doesn't work in Go, since the read syscall should return -1 in this case, with errstr set to "Hangup". |
The issue is that in the "accept" case, netFD.ctl represents listener's The "hangup" string is not handled on the Writing the "hangup" string to the |
Is this a dup of #11476, etc? I feel like a number of the failing |
Yes, but I don't have a proper fix for this issue yet. I propose that we skip this test until we figure out the right way to fix this issue. |
CL https://golang.org/cl/22513 mentions this issue. |
Updates #15464. Change-Id: If3221034bb10751c6fcf1fbeba401a879c18079f Reviewed-on: https://go-review.googlesource.com/22513 Run-TryBot: David du Colombier <0intro@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Not a Go 1.7 blocker, since Plan 9 isn't a first class port. I'd consider a CL for for 1.7, though, depending on what the fix ends up looking like. |
CL https://golang.org/cl/31270 mentions this issue. |
CL https://golang.org/cl/31271 mentions this issue. |
Previously, we used to write the "hangup" message to the TCP connection control file to be able to close a connection, while waking up the readers. The "hangup" message closes the TCP connection with a RST message. This is a problem when closing a connection consecutively to a write, because the reader may not have time to acknowledge the message before the connection is closed, resulting in loss of data. We use a "close" message, newly implemented in the Plan 9 kernel to be able to close a TCP connection gracefully with a FIN. Updates #15464. Change-Id: I2050cc72fdf7a350bc6c9128bae7d14af11e599c Reviewed-on: https://go-review.googlesource.com/31271 Run-TryBot: David du Colombier <0intro@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This version adds a TCP "close" message: http://9legacy.org/9legacy/patch/9-tcp-close.diff Updates golang/go#15464 Change-Id: I4866858338db4b408fd180b5d89560aa86049adc Reviewed-on: https://go-review.googlesource.com/31327 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL https://golang.org/cl/31393 mentions this issue. |
TestTransportRemovesDeadIdleConnections (CL 22492) is failing on Plan 9.
See https://build.golang.org/log/669586c471ca9095d6b6f2d0237e975e10792022
The text was updated successfully, but these errors were encountered: