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.

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

Fix shutdown crash when log/error callback is set #169

Merged
merged 2 commits into from
Jan 30, 2021

Conversation

phile314
Copy link
Contributor

The rd_kafka_destroy method may call back into Haskell to perform some logging. This is not allowed when attaching the finalizer
using Foreign.ForeignPtr. Hence we switch to Foreign.Concurrent, which allows RdKafka to call back into Haskell. We also disable
closing the consumer automatically in the finalizer, this should be done by the application by calling closeConsumer explicitly.

The rd_kafka_destroy method may call back into Haskell to perform some logging. This is not allowed when attaching the finalizer
using Foreign.ForeignPtr. Hence we switch to Foreign.Concurrent, which allows RdKafka to call back into Haskell. We also disable
closing the consumer automatically in the finalizer, this should be done by the application by calling `closeConsumer` explicitly.
@phile314
Copy link
Contributor Author

phile314 commented Jan 25, 2021

Using our test-suite I could reliably reproduce the error: a C finalizer called back into Haskell. crash using master. With this diff, everything works as it should and the application terminates properly.

@phile314
Copy link
Contributor Author

The CI errors seems to be because it uses a librdkafka version from March 2018, but rd_kafka_destroy_flags has been added in August 2018. From the git log, it seems version 0.11.6 (revision 849c066b559950b02e37a69256f0cb7b04381d0e) from October 2018 would suffice.

@AlexeyRaga AlexeyRaga merged commit ea326de into haskell-works:master Jan 30, 2021
@AlexeyRaga
Copy link
Member

Thanks a lot! It was a long standing issue...

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.

2 participants