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

panic due to index out of range if a request came in too early #32

Closed
kalbasit opened this issue Jul 15, 2016 · 3 comments
Closed

panic due to index out of range if a request came in too early #32

kalbasit opened this issue Jul 15, 2016 · 3 comments

Comments

@kalbasit
Copy link

kalbasit commented Jul 15, 2016

Could be related to #29 and #30

I have a server (gRPC, HTTP, and HTTPs) that is intended to run on Kubernetes, relevant code:

// code  in main() simplified.
var serverClosed chan bool
listenAddr := ":8091"
func main() {
    serverClosed = make(chan bool)
    srvListener, _ = net.Listen("tcp", listenAddr)
    mainServer := &http.Server{TLSConfig: &tls.Config{}} // tls configured
    healthCheckServer := &http.Server{}
    go startServer(mainServer, healthCheckServer, srvListener)

    // wait for the servet to exit
    // this is actually done because I have another goroutine that updates the certificate 
    // through let's encrypt and then closes srvListener, changes the TLSConfig on the
    // mainServer and calls startServer again. It's not visible here because the server is crashing
    // before that goroutine even run (it runs twice a day).
    <- serverClosed
}

func startServer(mainServer, healthCheckServer *http.Server, srvListener net.Listener) {
  log.Printf("starting the HTTP/HTTPS/gRPC server bound to %q", listenAddr)
  // create a new connection multiplexer.
  m := cmux.New(srvListener)
  // we first match on HTTP 1.1 methods.
  httpl := m.Match(cmux.HTTP1Fast())
  // if not matched, we assume that its TLS.
  tlsl := m.Match(cmux.Any())
  // start the mainServer
  go mainServer.Serve(tls.NewListener(tlsl, mainServer.TLSConfig))
  // start the healthCheckServer
  go healthCheckServer.Serve(httpl)
  // boot the connection multiplexer, this should return once the srvListener
  // is closed.
  m.Serve()

  select {
  case serverClosed <- true:
  default:
  }
}

Having a pod health check setup as follows:

          livenessProbe:
            tcpSocket:
              port: 8091
            initialDelaySeconds: 5
            timeoutSeconds: 1
          readinessProbe:
            httpGet:
              port: 8091
              path: /healthz
            initialDelaySeconds: 5
            timeoutSeconds: 1

Given the initialDelaySeconds=5, I assume the server has no time to setup all of the matchers and:

panic: runtime error: index out of range

goroutine 86 [running]:
panic(0xf203a0, 0xc820014010)
        /usr/local/go/src/runtime/panic.go:481 +0x3e6
github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.(*ptNode).match(0xc820405590, 0xc82051d6b8, 0x0, 0x8, 0x1, 0x0)
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/patricia.go:162 +0x1f0
github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.(*patriciaTree).matchPrefix(0xc82040ea00, 0x7f5a1e877b08, 0xc820272550, 0x40cfca)
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/patricia.go:52 +0x9f
github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.(*patriciaTree).(github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.matchPrefix)-fm(0x7f5a1e877b08, 0xc820272550, 0x7f5a1e8675b0)
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/matchers.go:37 +0x34
github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.matchersToMatchWriters.func1(0x7f5a1e8675b0, 0xc820022508, 0x7f5a1e877b08, 0xc820272550, 0xc820022508)
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/cmux.go:112 +0x32
github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.(*cMux).serve(0xc82046a200, 0x7f5a1e866e90, 0xc820022508, 0xc8204691a0, 0xc82051d700)
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/cmux.go:168 +0x33e
created by github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.(*cMux).Serve
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/cmux.go:158 +0x188

Changing initialDelaySeconds to 20 seconds solves the problem.

@kalbasit
Copy link
Author

not sure if it's related to how early is the connection at all, it seems to crash as soon as it gets a connection.

I applied #30 but it didn't work

diff --git a/vendor/github.com/soheilhy/cmux/patricia.go b/vendor/github.com/soheilhy/cmux/patricia.go
index 0099e38..a38cae0 100644
--- a/vendor/github.com/soheilhy/cmux/patricia.go
+++ b/vendor/github.com/soheilhy/cmux/patricia.go
@@ -22,8 +22,8 @@ import (
 // patriciaTree is a simple patricia tree that handles []byte instead of string
 // and cannot be changed after instantiation.
 type patriciaTree struct {
-       root *ptNode
-       buf  []byte // preallocated buffer to read data while matching
+       root     *ptNode
+       maxDepth int // max depth of the tree.
 }

 func newPatriciaTree(bs ...[]byte) *patriciaTree {
@@ -34,8 +34,8 @@ func newPatriciaTree(bs ...[]byte) *patriciaTree {
                }
        }
        return &patriciaTree{
-               root: newNode(bs),
-               buf:  make([]byte, max+1),
+               root:     newNode(bs),
+               maxDepth: max + 1,
        }
 }

@@ -48,13 +48,15 @@ func newPatriciaTreeString(strs ...string) *patriciaTree {
 }

 func (t *patriciaTree) matchPrefix(r io.Reader) bool {
-       n, _ := io.ReadFull(r, t.buf)
-       return t.root.match(t.buf[:n], true)
+       buf := make([]byte, t.maxDepth)
+       n, _ := io.ReadFull(r, buf)
+       return t.root.match(buf[:n], true)
 }

 func (t *patriciaTree) match(r io.Reader) bool {
-       n, _ := io.ReadFull(r, t.buf)
-       return t.root.match(t.buf[:n], false)
+       buf := make([]byte, t.maxDepth)
+       n, _ := io.ReadFull(r, buf)
+       return t.root.match(buf[:n], false)
 }

 type ptNode struct {
2016/07/15 07:06:58 starting the HTTP/HTTPS/gRPC server bound to ":8091"
panic: runtime error: index out of range

goroutine 87 [running]:
panic(0xf203a0, 0xc820014010)
        /usr/local/go/src/runtime/panic.go:481 +0x3e6
github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.(*ptNode).match(0xc82051d950, 0xc82054d388, 0x0, 0x8, 0x1, 0x0)
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/patricia.go:164 +0x1f0
github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.(*patriciaTree).matchPrefix(0xc8205596f0, 0x7fa00d2bcc28, 0xc8202766d0, 0x40cfca)
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/patricia.go:53 +0xd7
github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.(*patriciaTree).(github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.matchPrefix)-fm(0x7fa00d2bcc28, 0xc8202766d0, 0x7fa00d2a85b0)
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/matchers.go:37 +0x34
github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.matchersToMatchWriters.func1(0x7fa00d2a85b0, 0xc820022518, 0x7fa00d2bcc28, 0xc8202766d0, 0xc820022518)
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/cmux.go:112 +0x32
github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.(*cMux).serve(0xc8202a5000, 0x7fa00d2a7e90, 0xc820022518, 0xc82029b200, 0xc820559730)
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/cmux.go:168 +0x33e
created by github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux.(*cMux).Serve
        /go/src/github.com/publica-project/grpc-logger/vendor/github.com/soheilhy/cmux/cmux.go:158 +0x188

soheilhy added a commit that referenced this issue Jul 15, 2016
Bug #32 reported that there is an index out of range error. This
issue was introduced in 703b087.
@soheilhy
Copy link
Owner

Thanks for the bug report! Can you please check if #33 fixes the issue for you?

@kalbasit
Copy link
Author

#33 fixed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants