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: go1.6beta1, nil pointer in http.Client #13839

Closed
karmeye opened this issue Jan 6, 2016 · 9 comments
Closed

net/http: go1.6beta1, nil pointer in http.Client #13839

karmeye opened this issue Jan 6, 2016 · 9 comments
Milestone

Comments

@karmeye
Copy link

karmeye commented Jan 6, 2016

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x20 pc=0x58d4e8]

goroutine 39320 [running]:
net/http.(*persistConn).closeLocked(0xc8646c8840)
    /usr/local/go/src/net/http/transport.go:1451 +0x48
net/http.(*persistConn).close(0xc8646c8840)
    /usr/local/go/src/net/http/transport.go:1445 +0x7a
net/http.(*Transport).putIdleConn(0xc820386bb0, 0xc8646c8840, 0xc84fc4d788)
    /usr/local/go/src/net/http/transport.go:500 +0x434
net/http.(*Transport).getConn.func2.1(0xc841323aa0, 0xc820386bb0, 0x9db1c8)
    /usr/local/go/src/net/http/transport.go:629 +0x7f
created by net/http.(*Transport).getConn.func2
    /usr/local/go/src/net/http/transport.go:632 +0x70

Occurs sometimes under the following conditions

  • Using TLS and HTTP/2
  • Stress testing server with 500 http.Clients running in the same process.

Clients are running on OS X 10.11.2
Server is running on Linux

Looking at the source, it seems to be than conn is nil:

func (pc *persistConn) closeLocked() {
    pc.broken = true
    if !pc.closed {
        pc.conn.Close()    <---- conn nil?
        pc.closed = true
        close(pc.closech)
    }
    pc.mutateHeaderFunc = nil
}
@ianlancetaylor ianlancetaylor changed the title go1.6beta1, nil pointer in http.Client net/http: go1.6beta1, nil pointer in http.Client Jan 6, 2016
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Jan 6, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Jan 6, 2016

Have you run your program with the race detector? http://blog.golang.org/race-detector

@bradfitz
Copy link
Contributor

bradfitz commented Jan 7, 2016

I just read the code again at the version you're using (https://github.com/golang/go/blob/8db371b3d58a1c139f0854738f9962de05ca5d7a/src/net/http/transport.go) and see no way for pc or pc.conn to be nil.

Except perhaps if you're using the Transport.Dial hook and returning (nil, nil) from it.

Is your code available somewhere for me to look at?

@bradfitz
Copy link
Contributor

bradfitz commented Jan 7, 2016

Alternatively, are you able to reproduce with the latest code in the master branch?

@karmeye
Copy link
Author

karmeye commented Jan 7, 2016

Seems it was a race issue.

My stress test code is using a client library.
https://github.com/go-resty/resty/blob/master/client.go
In the execute() method, which is called concurrently, there were 2 issues:

// RACE 1, writing
if req.proxyURL != nil {
    c.transport.Proxy = http.ProxyURL(req.proxyURL)
} else if c.proxyURL != nil {
    c.transport.Proxy = http.ProxyURL(c.proxyURL)
} else {
    c.transport.Proxy = nil
}

// RACE 2, writing
c.httpClient.Transport = c.transport

// RACE reading transport/proxy ?
resp, err := c.httpClient.Do(req.RawRequest)

After protecting this whole section with a mutex I cannot reproduce the panic. (There were 2 other race conditions elsewhere as well but I think they were unrelated.)

A Dial hook is set in the method SetTimeout, but I was not calling it.

Is it possible that the GC can nil things if races occur?

Anyways, I'll close this issue 👍

@karmeye karmeye closed this as completed Jan 7, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Jan 7, 2016

Is it possible that the GC can nil things if races occur?

If you have races, anything is possible. Even very strange things. Regularly detecting races and fixing them should always be your top priority.

@karmeye karmeye reopened this Jan 11, 2016
@karmeye karmeye closed this as completed Jan 11, 2016
@karmeye
Copy link
Author

karmeye commented Jan 13, 2016

This is still an issue.

This is most definitely related to unexpected connection shutdowns from the server. If the connection is being closed while the client is reading/writing.

For example, in the case above (using 1.6 beta1), it stopped appearing when extending the Read and Write deadlines on the server in ConnState http.StateActive as seen here #13925 (comment)

Now using latest using latest code in the master branch (771da53) it is still appearing even with the change above.

In this case it was when the server got Killed by the system (memory pressure). Probably the same thing again, the client expected the connection to be open and was reading/writing to it.

I had 3 programs running on 3 different machines with several clients running in goroutines, and they all got the same panic at the same time with exactly the same stack trace as above (although different line numbers now):

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x20 pc=0x57aacc]

goroutine 1084538 [running]:
net/http.(*persistConn).closeLocked(0xc822551450, 0x7f95b1e55028, 0xc820010dd0)
    /usr/local/go/src/net/http/transport.go:1505 +0xac
net/http.(*persistConn).close(0xc822551450, 0x7f95b1e55028, 0xc820010dd0)
    /usr/local/go/src/net/http/transport.go:1496 +0x92
net/http.(*Transport).putOrCloseIdleConn(0xc8200ae2c0, 0xc822551450)
    /usr/local/go/src/net/http/transport.go:478 +0x5d
net/http.(*Transport).getConn.func2.1(0xc8214d3860, 0xc8200ae2c0, 0x98ec88)
    /usr/local/go/src/net/http/transport.go:656 +0x7f
created by net/http.(*Transport).getConn.func2
    /usr/local/go/src/net/http/transport.go:659 +0x70

I am now using a Transport.Dial hook which is wrapping and returning the values of net.DialTimeout(network, addr, ioTimeout).

@karmeye karmeye reopened this Jan 13, 2016
@bradfitz
Copy link
Contributor

I don't see how such a case is possible without data races or other bugs. Are you sure your DialTimeout hook isn't returning (nil, nil)? Can you share your code and instructions to reproduce?

@bradfitz
Copy link
Contributor

Actually, I have a potential lead now.

@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Jan 13, 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

4 participants