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.

Sign up for GitHub

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

Jump to bottom

Panic in gzip compressor, possibly due to incorrect concurrent use #8171

Open
jhump opened this issue Mar 14, 2025 · 1 comment · May be fixed by #8178
Open

Panic in gzip compressor, possibly due to incorrect concurrent use #8171

jhump opened this issue Mar 14, 2025 · 1 comment · May be fixed by #8178
Assignees
@dfawley
Labels
Area: RPC Features Includes Compression, Encoding, Attributes/Metadata, Interceptors. Type: Bug

Comments

@jhump
Copy link
Member

jhump commented Mar 14, 2025

In a depend-a-bot PR, to upgrade to the latest v1.71.0 version of this library (link), CI encountered a panic inside the gRPC runtime. Looking at the stack-trace, it seems impossible: it's hitting a nil pointer dereference on a field named hl, but the preceding stack frame sets hl to a non-nil pointer.

The only possible way I can think of this happening is if a compressor is incorrectly used concurrently -- like another goroutine resetting this compressor in between the write and subsequent read of this field. And that could happen if it is accidentally returned to a sync.Pool before we are actually finished with it.

The only commit I could see between 1.70.0 and 1.71.0 that might be a culprit is #7918. I am still analyzing the code and have also been re-running this with the race detector enabled, hoping it could highlight a smoking gun. But I do know sync.Pool behaves a little differently when race detection is enabled. So far, it has not found anything, but I was able to reproduce the issue with a different panic (described further below).

Here's the full stack trace from the panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x599edc]

goroutine 570 [running]:
compress/flate.(*decompressor).huffSym(0xc0003dc008, 0x0)
	/opt/hostedtoolcache/go/1.23.7/x64/src/compress/flate/inflate.go:713 +0x1c
compress/flate.(*decompressor).huffmanBlock(0xc0003dc008?)
	/opt/hostedtoolcache/go/1.23.7/x64/src/compress/flate/inflate.go:495 +0x45
compress/flate.(*decompressor).nextBlock(0xc0003dc008)
	/opt/hostedtoolcache/go/1.23.7/x64/src/compress/flate/inflate.go:320 +0xd6
compress/flate.(*decompressor).Read(0xc0003dc008, {0xc001080000, 0x8000, 0x1?})
	/opt/hostedtoolcache/go/1.23.7/x64/src/compress/flate/inflate.go:348 +0x5b
compress/gzip.(*Reader).Read(0xc0003da848, {0xc001080000, 0x8000, 0x8000})
	/opt/hostedtoolcache/go/1.23.7/x64/src/compress/gzip/gunzip.go:252 +0xa2
google.golang.org/grpc/encoding/gzip.(*reader).Read(0xc00012cf60, {0xc001080000?, 0xc00007e[54](https://github.com/connectrpc/conformance/actions/runs/13846074270/job/38744663478?pr=981#step:7:55)0?, 0x8000?})
	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.71.0/encoding/gzip/gzip.go:107 +0x27
io.(*LimitedReader).Read(0xc0013acc30, {0xc001080000?, 0x18?, 0xff?})
	/opt/hostedtoolcache/go/1.23.7/x64/src/io/io.go:479 +0x43
google.golang.org/grpc/mem.ReadAll({0xba0[56](https://github.com/connectrpc/conformance/actions/runs/13846074270/job/38744663478?pr=981#step:7:57)0, 0xc0013acc30}, {0xba5000, 0xc00007e540})
	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.71.0/mem/buffer_slice.go:260 +0x249
google.golang.org/grpc.decompress({0xba6180, 0xc0000d80f0}, {0xc000f14f90?, 0x0?, 0x4fce0a?}, {0x0?, 0x0?}, 0x32000, {0xba5000, 0xc00007e540})
	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.71.0/rpc_util.go:873 +0x387
google.golang.org/grpc.recvAndDecompress(0xc000d8d380, {0xba11a0, 0xc0011fbf20}, {0x0, 0x0}, 0x32000, 0x0, {0xba6180, 0xc0000d80f0}, 0x1)
	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.71.0/rpc_util.go:835 +0x2b7
google.golang.org/grpc.recv(0x10?, {0x7f08f7edff88, 0x1018980}, {0xba11a0?, 0xc0011fbf20?}, {0x0?, 0x0?}, {0xa5d5c0, 0xc001091ef0}, 0x32000, ...)
	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.71.0/rpc_util.go:902 +0xab
google.golang.org/grpc.(*serverStream).RecvMsg(0xc00082f340, {0xa5d5c0, 0xc001091ef0})
	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.71.0/stream.go:1760 +0x195
connectrpc.com/conformance/internal/gen/proto/go/connectrpc/conformance/v1._ConformanceService_ServerStream_Handler({0xa5cc00, 0x1018980}, {0xba8a10, 0xc00082f340})
	/home/runner/work/conformance/conformance/internal/gen/proto/go/connectrpc/conformance/v1/service_grpc.pb.go:464 +0x54
connectrpc.com/conformance/internal/app/grpcserver.serverNameStreamInterceptor({0xa5cc00, 0x1018980}, {0xba8a10, 0xc00082f340}, 0x9b1680?, 0xaff740)
	/home/runner/work/conformance/conformance/internal/app/grpcserver/impl.go:421 +0x62
google.golang.org/grpc.(*Server).processStreamingRPC(0xc0000be800, {0xba6ba8, 0xc000d8d320}, 0xc0011fbf20, 0xc0002336e0, 0xfe8380, 0x0)
	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.71.0/server.go:1702 +0x124a
google.golang.org/grpc.(*Server).handleStream(0xc0000be800, {0xba7050, 0xc0002c49c0}, 0xc0011fbf20)
	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.71.0/server.go:1819 +0xb69
google.golang.org/grpc.(*Server).serveStreams.func2.1()
	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.71.0/server.go:1035 +0x7f
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 569
	/home/runner/go/pkg/mod/google.golang.org/grpc@v1.71.0/server.go:1046 +0x125

The second panic I've seen that is also inside the gRPC runtime and related to gzip compression is a slice bounds out of range issue. I have not dug into the stack trace as deeply as the one above, but it smells like it could also be a concurrent modification issue.

panic: runtime error: slice bounds out of range [51:0]

goroutine 941 [running]:
compress/flate.(*decompressor).Read(0x14000d82008, {0x14000c28000, 0x8000, 0x10449ec10?})
	/Users/jhumphries/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.6.darwin-arm64/src/compress/flate/inflate.go:339 +0x1d4
compress/gzip.(*Reader).Read(0x1400059bb88, {0x14000c28000, 0x8000, 0x8000})
	/Users/jhumphries/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.6.darwin-arm64/src/compress/gzip/gunzip.go:252 +0x80
google.golang.org/grpc/encoding/gzip.(*reader).Read(0x1400009eab0, {0x14000c28000?, 0x140001244c0?, 0x8000?})
	/Users/jhumphries/go/pkg/mod/google.golang.org/grpc@v1.71.0/encoding/gzip/gzip.go:107 +0x2c
io.(*LimitedReader).Read(0x140001818f0, {0x14000c28000?, 0x18?, 0x1400009eaff?})
	/Users/jhumphries/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.6.darwin-arm64/src/io/io.go:479 +0x50
google.golang.org/grpc/mem.ReadAll({0x104b62248, 0x140001818f0}, {0x104b66980, 0x140001244c0})
	/Users/jhumphries/go/pkg/mod/google.golang.org/grpc@v1.71.0/mem/buffer_slice.go:260 +0x234
google.golang.org/grpc.decompress({0x104b67a80, 0x1400015e0f0}, {0x1400038f430?, 0x0?, 0x0?}, {0x0?, 0x0?}, 0x32000, {0x104b66980, 0x140001244c0})
	/Users/jhumphries/go/pkg/mod/google.golang.org/grpc@v1.71.0/rpc_util.go:873 +0x32c
google.golang.org/grpc.recvAndDecompress(0x140018a1500, {0x104b62a68, 0x14001e88e40}, {0x0, 0x0}, 0x32000, 0x0, {0x104b67a80, 0x1400015e0f0}, 0x1)
	/Users/jhumphries/go/pkg/mod/google.golang.org/grpc@v1.71.0/rpc_util.go:835 +0x1e4
google.golang.org/grpc.recv(0x14000480c60?, {0x10bde3ff8, 0x104fc69c0}, {0x104b62a68?, 0x14001e88e40?}, {0x0?, 0x0?}, {0x104b02e40, 0x140006843c0}, 0x65?, ...)
	/Users/jhumphries/go/pkg/mod/google.golang.org/grpc@v1.71.0/rpc_util.go:902 +0x78
google.golang.org/grpc.(*serverStream).RecvMsg(0x140003b56c0, {0x104b02e40, 0x140006843c0})
	/Users/jhumphries/go/pkg/mod/google.golang.org/grpc@v1.71.0/stream.go:1760 +0x118
connectrpc.com/conformance/internal/gen/proto/go/connectrpc/conformance/v1._ConformanceService_ServerStream_Handler({0x104b02480, 0x104fc69c0}, {0x104b6a2d0, 0x140003b56c0})
	/Users/jhumphries/src/conformance/internal/gen/proto/go/connectrpc/conformance/v1/service_grpc.pb.go:464 +0x5c
connectrpc.com/conformance/internal/app/grpcserver.serverNameStreamInterceptor({0x104b02480, 0x104fc69c0}, {0x104b6a2d0, 0x140003b56c0}, 0x104a56480?, 0x104b5d740)
	/Users/jhumphries/src/conformance/internal/app/grpcserver/impl.go:421 +0x68
google.golang.org/grpc.(*Server).processStreamingRPC(0x14000288000, {0x104b684a8, 0x140018a14a0}, 0x14001e88e40, 0x14000185740, 0x104f90360, 0x0)
	/Users/jhumphries/go/pkg/mod/google.golang.org/grpc@v1.71.0/server.go:1702 +0xea4
google.golang.org/grpc.(*Server).handleStream(0x14000288000, {0x104b688a8, 0x14001d7e4e0}, 0x14001e88e40)
	/Users/jhumphries/go/pkg/mod/google.golang.org/grpc@v1.71.0/server.go:1819 +0x8e4
google.golang.org/grpc.(*Server).serveStreams.func2.1()
	/Users/jhumphries/go/pkg/mod/google.golang.org/grpc@v1.71.0/server.go:1035 +0x84
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 940
	/Users/jhumphries/go/pkg/mod/google.golang.org/grpc@v1.71.0/server.go:1046 +0x13c
@jhump
Copy link
Member Author

jhump commented Mar 14, 2025 •
edited
Loading

I do see a possible issue: in gzip.reader.Read, it eagerly returns the underlying gzip reader back to a pool on io.EOF. However, if the caller tries to read again (after EOF), it will hit this issue because it will end up trying to call that gzip reader's Read method even though it no longer "belongs" to the calling goroutine and may have been used concurrently/reset.

It's possible that the new changes in the mentioned PR might end up making a foillow-up Read call after EOF is returned, and that could be tickling what appears to be a pre-existing condition in the gzip encoding.

This was referenced Mar 14, 2025
Closed
Closed
@dfawley dfawley linked a pull request Mar 17, 2025 that will close this issue
Open
@purnesh42H purnesh42H added the Area: RPC Features Includes Compression, Encoding, Attributes/Metadata, Interceptors. label Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dfawley dfawley

Labels
Area: RPC Features Includes Compression, Encoding, Attributes/Metadata, Interceptors. Type: Bug
Projects
None yet
Milestone
No milestone
3 participants
@jhump @purnesh42H @dfawley