Skip to content

do not panic during chunk dispatching if consumer suddenly closed#393

Merged
Gsantomaggio merged 4 commits into
rabbitmq:mainfrom
rsperl:panic-chunk-dispatching
May 7, 2025
Merged

do not panic during chunk dispatching if consumer suddenly closed#393
Gsantomaggio merged 4 commits into
rabbitmq:mainfrom
rsperl:panic-chunk-dispatching

Conversation

@rsperl

@rsperl rsperl commented May 7, 2025

Copy link
Copy Markdown
Contributor

We've seen repeated panics when a consumer suddenly goes away:

panic: send on closed channel

goroutine 605 [running]:
github.com/rabbitmq/rabbitmq-stream-go-client/pkg/stream.(*Client).handleDeliver(0x140004d4000, 0x1400099c000)
	/root/go/pkg/mod/github.com/rabbitmq/rabbitmq-stream-go-client@v1.5.3/pkg/stream/server_frame.go:410 +0xaa0
github.com/rabbitmq/rabbitmq-stream-go-client/pkg/stream.(*Client).handleResponse(0x140004d4000)
	/root/go/pkg/mod/github.com/rabbitmq/rabbitmq-stream-go-client@v1.5.3/pkg/stream/server_frame.go:84 +0x294
created by github.com/rabbitmq/rabbitmq-stream-go-client/pkg/stream.(*Client).connect in goroutine 601
	/root/go/pkg/mod/github.com/rabbitmq/rabbitmq-stream-go-client@v1.5.3/pkg/stream/client.go:196 +0x86c

This PR ensures that the consumer.chunkForConsumer channel is open before dispatching a chunk to it.

@Gsantomaggio

Copy link
Copy Markdown
Member

Thank you, @rsperl. That's interesting. I had never encountered this condition before, and the fix makes sense to me.
@hiimjako WDYT?

@hiimjako

hiimjako commented May 7, 2025

Copy link
Copy Markdown
Collaborator

It never happened to me too. It is strange to me because the client is set to close before closing the consumer.chunkForConsumer, so it should not happen.
But anyway, it makes sense to me too.

Comment thread pkg/stream/server_frame.go Outdated
@Gsantomaggio

Copy link
Copy Markdown
Member

@rsperl
Is there a way to reproduce this problem?
Did you have this problem? If yes, can you please add more info?
Thank you

Comment thread pkg/stream/server_frame.go Outdated
@rsperl

rsperl commented May 7, 2025

Copy link
Copy Markdown
Contributor Author

@rsperl Is there a way to reproduce this problem? Did you have this problem? If yes, can you please add more info? Thank you

Unfortunately, i don't have an isolated example, only a very complex one in code I cannot share, though it was fairly reproducable. We have a rabbitmq proxy server using this library, and when we have multiple consumers processing many events, suddenly stopping the consumers (ctrl-c or a kill -9) will cause a panic in the proxy server with the stack trace above.

@Gsantomaggio

Copy link
Copy Markdown
Member

We have a rabbitmq proxy server using this library, and when we have multiple consumers processing many events, suddenly stopping the consumers

It seems an interesting use case!

Out of curiosity: How is it going with the library (except for this bug)?

@rsperl

rsperl commented May 7, 2025

Copy link
Copy Markdown
Contributor Author

We have a rabbitmq proxy server using this library, and when we have multiple consumers processing many events, suddenly stopping the consumers

It seems an interesting use case!

Out of curiosity: How is it going with the library (except for this bug)?

I don't, but we only started using streams a couple of months ago (and we are using rabbitmq 3.12 btw).

@Gsantomaggio Gsantomaggio merged commit dd20421 into rabbitmq:main May 7, 2025
@Gsantomaggio Gsantomaggio added this to the 1.5.5 milestone May 8, 2025
hiimjako added a commit that referenced this pull request Jul 3, 2025
@hiimjako hiimjako mentioned this pull request Jul 3, 2025
Gsantomaggio added a commit that referenced this pull request Jul 3, 2025
Co-authored-by: Gabriele Santomaggio <G.santomaggio@gmail.com>
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

Successfully merging this pull request may close these issues.

3 participants