valyala / fasthttp

Fast HTTP package for Go. Tuned for high performance. Zero memory allocations in hot paths. Up to 10x faster than net/http
MIT License
21.95k stars 1.76k forks source link

mustDiscard() panics and the whole server crashes #1442

Open mostafatalebi opened 2 years ago

mostafatalebi commented 2 years ago

In fasthttp, in file header.go, this function:

func mustDiscard(r *bufio.Reader, n int) {
    if _, err := r.Discard(n); err != nil {
        panic(fmt.Sprintf("bufio.Reader.Discard(%d) failed: %v", n, err))
    }
}

sometimes, under high concurrency, panics with error panic: bufio.Reader.Discard(517) failed: read tcp4 ADDR-1->:8081->ADDR-2:30116: i/o timeout and the server crashes.

Why it is not handled by error? Because the panic is on the side of the worker, and the thrown panic cannot be recovered by any of the RouterPanic or Recovery middleware. What should be done here?

li-jin-gou commented 2 years ago

It looks like it could return err instead of panic here?

erikdubbelboer commented 2 years ago

Do you have a stacktrace for me? The reason this panics is that it shouldn't be possible to happen. It should always call Discard with a number of bytes it already read in which case it should never do any IO.

mostafatalebi commented 2 years ago

Do you have a stacktrace for me? The reason this panics is that it shouldn't be possible to happen. It should always call Discard with a number of bytes it already read in which case it should never do any IO.

Yes, here is the stacktrace:

panic: bufio.Reader.Discard(10196) failed: read tcp4 addr1:8081->addr2:10394: i/o timeout

2022-11-18T02:48:03.814+03:30   goroutine 4875549 [running]:

2022-11-18T02:48:03.814+03:30   github.com/valyala/fasthttp.mustDiscard(0xc00de14600, 0xc00b98b800)

2022-11-18T02:48:03.814+03:30   /repo/vendor/github.com/valyala/fasthttp/header.go:3244 +0xba

2022-11-18T02:48:03.814+03:30   github.com/valyala/fasthttp.(*RequestHeader).tryRead(0xc00de14600, 0xc029758540, 0x23b0)

2022-11-18T02:48:03.814+03:30   /repo/vendor/github.com/valyala/fasthttp/header.go:2070 +0x371

2022-11-18T02:48:03.814+03:30   github.com/valyala/fasthttp.(*RequestHeader).readLoop(0xc00702c2c0, 0xc029758540, 0x1)

2022-11-18T02:48:03.814+03:30   /repo/vendor/github.com/valyala/fasthttp/header.go:1971 +0x4d

2022-11-18T02:48:03.814+03:30   github.com/valyala/fasthttp.(*RequestHeader).Read(...)

2022-11-18T02:48:03.814+03:30   /repo/vendor/github.com/valyala/fasthttp/header.go:1962

2022-11-18T02:48:03.814+03:30   github.com/valyala/fasthttp.(*Server).serveConn(0xc0000b0fc0, {0x13c7660, 0xc041b826c0})

2022-11-18T02:48:03.814+03:30   /repo/vendor/github.com/valyala/fasthttp/server.go:2180 +0x925

2022-11-18T02:48:03.814+03:30   github.com/valyala/fasthttp.(*workerPool).workerFunc(0xc0000a60a0, 0xc02bc06da0)

2022-11-18T02:48:03.814+03:30   /repo/vendor/github.com/valyala/fasthttp/workerpool.go:224 +0xa9

2022-11-18T02:48:03.814+03:30   github.com/valyala/fasthttp.(*workerPool).getCh.func1()

2022-11-18T02:48:03.814+03:30   /repo/vendor/github.com/valyala/fasthttp/workerpool.go:196 +0x38

2022-11-18T02:48:03.814+03:30   created by github.com/valyala/fasthttp.(*workerPool).getCh

2022-11-18T02:48:03.814+03:30   /repo/vendor/github.com/valyala/fasthttp/workerpool.go:195 +0x1b5
erikdubbelboer commented 2 years ago

Which version of fasthttp are you using? Can you please update to the latest version. Unless you are using the RequestCtx, Request or RequestHeaders after your handler has returned, it's impossible for mustDiscard to panic, as it will always discard bytes it has already buffered (which will cause no network read).

Is it possible that you are using RequestCtx, Request, RequestHeaders or any of their members after your handler has returned?

mostafatalebi commented 2 years ago

Already the latest version. We have not hit that again. But, the thing about RequestCtx, I look into it