When should a RoundTripper close its connection?

91 views
Skip to first unread message

trevo...@gmail.com

unread,
Oct 18, 2017, 4:26:07 PM10/18/17
to golang-nuts
I'm using httputil.ReverseProxy with an http.RoundTripper of my own implementation that uses an ssh.Channel as a transport. My RoundTrip method looks approximately like this:

func (c SSHConnection) RoundTrip(req *http.Request) (*http.Response, error) {
ch, err := c.GetChannel()
if err != nil {
return nil, errors.New("couldn't open forwarded-tcpip channel: " + err.Error())
}
// defer ch.Close()
err = req.Write(ch)
if err != nil {
return nil, errors.New("couldn't send request: " + err.Error())
}

return http.ReadResponse(bufio.NewReader(ch), req)
}

func (c SSHConnection) GetChannel() (ssh.Channel, error) {
ch, req, err := c.Conn.OpenChannel("forwarded-tcpip", msg)
if err != nil {
return nil, err
}
go ssh.DiscardRequests(req)
return ch, nil
}

Notice the commented-out defer ch.Close(). Evidently it's wrong to close the connection here, because if I do, the response body is sometimes empty, apparently due to a race condition between the HTTP proxy's reading of the body and this closing of the SSH channel.

Assuming, for now, that I don't care to do keep-alive, when can I close the ssh.Channel? If I don't, every request starts a new goroutine (because of go ssh.DiscardRequests(req)), so I leak a goroutine on every HTTP requests until the SSH connection is closed.

Frits van Bommel

unread,
Nov 3, 2017, 3:04:10 PM11/3/17
to golang-nuts
The best way is probably to replace the Response.Body before returning the result of http.ReadResponse() with an io.ReadCloser that also closes the underlying channel.

Replace the end of RoundTrip() with something like this (untested) code:
    resp := http.ReadResponse(bufio.NewReader(ch), req)
    resp.Body = channelCloser{ resp.Body, ch }
    return resp
}

type channelCloser struct {
io.ReadCloser
ch ssh.Channel
}
func (cc *channelCloser) Close() error {
    err := cc.ReadCloser.Close()
    err2 := cc.ch.Close()
    if err == nil {
        err = err2
    }
    return err
}

This way the channel is closed when the body of the response is closed, and no sooner.
Reply all
Reply to author
Forward
0 new messages