Panic on server shutdown

[mpldr]

I encounter this panic when shutting down the server.

The steps are:

• start server
• get index page with firefox
• send sigint (invalidates the context)
• see panic

Server setup/shutdown:

	lis, err := net.Listen("tcp", "localhost:8080")
	if err != nil {
		glog.Errorf("failed to listen on %s: %v", ":8080", err)
		return
	}
	srv := &fasthttp.Server{
		Handler:                      handler.RootHandler,
		MaxConnsPerIP:                1,
		MaxRequestBodySize:           260 * 1024 * 1024,
		DisablePreParseMultipartForm: true,
		StreamRequestBody:            true,
		TLSConfig:                    &tls.Config{},
	}
	httpErr := make(chan error)
	go func() {
		httpErr <- srv.Serve(lis)
	}()

	select {
	case err = <-httpErr:
		if err != nil {
			glog.Errorf("server encoutered an error. not good: %v", err) // TODO: might want to rephrase this
		}
		errchan <- err
		fmt.Println(err)
	case <-ctx.Done():
		ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
		srv.ShutdownWithContext(ctx)
		cancel()
		return
	}

The panic I get is:

panic: BUG: negative per-ip counter=-1 for ip=2130706433

goroutine 6 [running]:
github.com/valyala/fasthttp.(*perIPConnCounter).Unregister(0xc0001660e8, 0x68cd0?)
	github.com/valyala/[email protected]/peripconn.go:35 +0x179
github.com/valyala/fasthttp.(*perIPConn).Close(0xc00030e000)
	github.com/valyala/[email protected]/peripconn.go:70 +0x4d
github.com/valyala/fasthttp.(*Server).closeIdleConns(0xc000166000)
	github.com/valyala/[email protected]/server.go:2904 +0x10f
github.com/valyala/fasthttp.(*Server).ShutdownWithContext(0xc000166000, {0x673328, 0xc00011a600})
	github.com/valyala/[email protected]/server.go:1907 +0x272
main.httpListener({0x6732b8, 0xc000090140}, 0x0?, 0xc00010e000)
	mpldr.codes/tarbash/main.go:71 +0x408
main.main.func1()
	mpldr.codes/tarbash/main.go:32 +0xcd
created by main.main
	mpldr.codes/tarbash/main.go:30 +0x111

At times I also get this:

^Cpanic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x5ae6c4]

goroutine 22 [running]:
github.com/valyala/fasthttp.(*perIPConn).Close(0xc000110080)
	github.com/valyala/[email protected]/peripconn.go:69 +0x24
github.com/valyala/fasthttp.(*workerPool).workerFunc(0xc000120140, 0xc0001100c0)
	github.com/valyala/[email protected]/workerpool.go:238 +0x374
github.com/valyala/fasthttp.(*workerPool).getCh.func1()
	github.com/valyala/[email protected]/workerpool.go:196 +0x38
created by github.com/valyala/fasthttp.(*workerPool).getCh
	github.com/valyala/[email protected]/workerpool.go:195 +0x1b0

But the latter is only sometimes.

[mpldr]

Not sure if the first even has to be a panic, to be honest. Looks like nothing but a sanity check to me.

[li-jin-gou]

hello @mpldr Can you provide a complete runnable code ? I would like to try to reproduce this problem in my computer.

[mpldr]

Sure, here you go: http://0x0.st/Hs5T.tzst

It's not yet part of any repo, so a tarball is the best I can offer you right away :)

[erikdubbelboer]

The problem is that Shutdown calls closeIdleConns. Which calls Close on all connections here:

fasthttp/server.go

Line 2903 in 559d536

_ = c.Close()

But then when the worker goroutine stops it also calls Close on the connection here:

fasthttp/workerpool.go

Line 238 in 404c8a8

_ = c.Close()

We should somehow stop one of them from happening. I think closeIdleConns needs to call Close to stop the read in serveConn. So we'll need some way for workerFunc not to close the connection. I'm not sure in which case this close is needed.

I'm afraid I don't have time to investigate this further right now. I'll have another look in the future.

[mpldr]

Thinking of it, I more and more think this shouldn't be a panic but instead handled gracefully™ (which might mean discarding after logging in this case)

[erikdubbelboer]

Should be fixed in #1526

[mpldr]

As mentioned in #1526 it now doesn't panic anymore. The underlying double close is not fixed. So not sure if this ticket should be renamed.