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

net/http/httptest: investigate removing the net.Conn.Close goroutine for Go 1.7 #14291

Closed
bradfitz opened this issue Feb 10, 2016 · 2 comments
Closed
Milestone

Comments

@bradfitz
Copy link
Contributor

From an updated comment I'm writing in net/http/httptest/server.go:

func (s *Server) closeConn(c net.Conn) {
        if runtime.GOOS == "plan9" {
                // Go's Plan 9 net package isn't great at unblocking reads when
                // their underlying TCP connections are closed.  Don't trust
                // that that the ConnState state machine will get to
                // StateClosed. Instead, just go there directly. Plan 9 may leak
                // resources if the syscall doesn't end up returning. Oh well.
                s.forgetConn(c)
        }

        // Somewhere in the chaos of https://golang.org/cl/15151 we found that  
        // some types of conns were blocking in Close too long (or deadlocking?)
        // and we had to call Close in a goroutine. I (bradfitz) forget what                                                                                                       
        // that was at this point, but I suspect it was *tls.Conns, which                                                                                                          
        // were later fixed in https://golang.org/cl/18572, so this goroutine                                                                                                      
        // is _probably_ unnecessary now. But it's too late in Go 1.6 too remove                                                                                                   
        // it with confidence.                                                                                                                                                     
        // TODO(bradfitz): try to remove it for Go 1.7.                                                                                                                            
        go c.Close()
}
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Feb 10, 2016
…to close

httptest.Server was rewritten during Go 1.6, but
CloseClientConnections was accidentally made async in the rewrite and
not caught due to lack of tests.

Restore the Go 1.5 behavior and add tests.

Fixes #14290
Updates #14291

Change-Id: I14f01849066785053ccca2373931bc82d78c0a13
Reviewed-on: https://go-review.googlesource.com/19432
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Mar 31, 2017
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

2 participants