caddy+QUIC crash on SIGHUP

[bprodoehl]

1. What version of Caddy are you using (caddy -version)?

Caddy 0.10.6

2. What are you trying to do?

It's crashing on exit.

3. What is your entire Caddyfile?

http://sub.mydomain.com {
  redir https://sub.mydomain.com{uri}
}
https://sub.mydomain.com {
    gzip
    tls /etc/caddy/certs/sub.mydomain.com.crt /etc/caddy/certs/sub.mydomain.com.key {
        protocols tls1.0 tls1.2
    }
        proxy / 172.17.0.2:80 {
          header_upstream Host {host}
          header_upstream X-Real-IP {remote}
          header_upstream X-Forwarded-For {remote}
          header_upstream X-Forwarded-Proto {scheme}
        }
}

4. How did you run Caddy (give the full command and describe the execution environment)?

/usr/bin/caddy -quic --conf /etc/caddy/conf/Caddyfile --log stdout

5. Please paste any relevant HTTP request(s) here.

6. What did you expect to see?

7. What did you see instead (give full error messages and/or log)?

2017/08/03 15:06:28 [INFO] SIGHUP: Hanging up
2017/08/03 15:06:28 http: Server closed
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x86a1e8]

goroutine 26 [running]:
github.com/mholt/caddy/vendor/github.com/lucas-clemente/quic-go/h2quic.(*Server).Close(0x0, 0x0, 0x0)
    src/github.com/mholt/caddy/vendor/github.com/lucas-clemente/quic-go/h2quic/server.go:244 +0x38
github.com/mholt/caddy/caddyhttp/httpserver.(*Server).Serve(0xc420051680, 0xe23f40, 0xc42005e1a8, 0x0, 0x0)
    src/github.com/mholt/caddy/caddyhttp/httpserver/server.go:309 +0xf2
github.com/mholt/caddy.startServers.func1.1(0xc420051740, 0xe24b40, 0xc420051680, 0xe23f40, 0xc42005e1a8, 0xc42005bad0)
    src/github.com/mholt/caddy/caddy.go:669 +0x49
created by github.com/mholt/caddy.startServers.func1
    src/github.com/mholt/caddy/caddy.go:671 +0xac

8. How can someone who is starting from scratch reproduce the bug as minimally as possible?

Should be able to bring Caddy up, do some QUIC requests to it, send SIGHUP, and watch it probably crash on exit. I don't know if this is a bug in lucas-clemente/quic-go or if its a bug in how Caddy is using it.

[mholt]

Line 244 in h2quic/server.go is s.listenerMutex.Lock() - so s must be nil - that's weird.

@marten-seemann Do you know, are we using it wrong? We close the quic server here using s.quicServer.Close().

[marten-seemann]

That looks weird.
I see that the s.quicServer is only set if HTTP2 && QUIC here, but closed if QUIC here. Maybe we should replace the last one by if s.quicServer != nil.

Does that solve the problem?

[mholt]

I was just thinking the same thing: the conditions should match, or maybe change to check for the existence of s.quicServer directly.

I'm actually having trouble reproducing this problem, even with a live domain and real certs.

But I did reproduce it once. It seems to be sporadic. But I'll try to verify if changing the condition fixes it.

[mholt]

Was able to make it happen consistently, and verified that the condition is at fault. QUIC may still be true even if that particular server doesn't have TLS enabled; forgot that the initial condition in the other function has a parent condition of TLSConfig != nil. Thanks for the report!

[bprodoehl]

No problem. Thanks for the awesome project and rapid response!