Skip to content

Fix kafka consumer not destroyed in close#540

Open
ikeberlein wants to merge 1 commit intoarnaud-lb:6.xfrom
ikeberlein:FixConsumerNotDestroyedOnClose
Open

Fix kafka consumer not destroyed in close#540
ikeberlein wants to merge 1 commit intoarnaud-lb:6.xfrom
ikeberlein:FixConsumerNotDestroyedOnClose

Conversation

@ikeberlein
Copy link
Copy Markdown
Contributor

@ikeberlein ikeberlein commented Jun 6, 2023

Not destroying underlying kafka consumer in close() method leads to unclean librdkafka shutdown and very often to segfaults.

@ikeberlein ikeberlein closed this Jun 6, 2023
@ikeberlein ikeberlein reopened this Jun 6, 2023
@ikeberlein ikeberlein marked this pull request as ready for review June 6, 2023 13:42
@ikeberlein ikeberlein closed this Jun 6, 2023
@ikeberlein ikeberlein reopened this Jun 6, 2023
@ikeberlein ikeberlein closed this Jun 6, 2023
@ikeberlein ikeberlein reopened this Jun 6, 2023
@ikeberlein ikeberlein closed this Jun 6, 2023
@ikeberlein ikeberlein reopened this Jun 6, 2023
@ikeberlein ikeberlein force-pushed the FixConsumerNotDestroyedOnClose branch from 73dc305 to 24e0e9c Compare January 12, 2024 09:27
@arnaud-lb
Copy link
Copy Markdown
Owner

Normally the rk handle is destroyed when the KafkaConsumer object itself is destroyed, so I don't think we need to close it explicitly here. However maybe we don't destroy various resources in the right order when the rk is closed later?

Do you have a reproducer?

@plehatron
Copy link
Copy Markdown

plehatron commented Dec 11, 2024

Hey @arnaud-lb, I prepared and reported the reproducer in #571. The issue includes details about the bug this PR fixes.

@mattiabasone
Copy link
Copy Markdown

Hey! Do you happen to have any news on this @arnaud-lb?
I'm experiencing the same issue during PHPUnit tests. In the CI, I'm using Ubuntu 24.04 and the shivammathur/setup-php GitHub action for setting up the environment with PHP 8.3.25 and rdkafka 6.0.5

@takeit
Copy link
Copy Markdown

takeit commented Nov 27, 2025

@arnaud-lb same issue here, could this be merged, pls? it's been over 2 years for 1 line to be merged

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.

5 participants