Skip to content

Fix crash when new topic is not created. #725

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

Merged
merged 1 commit into from
Nov 26, 2019

Conversation

elektito
Copy link

I found that the following code caused a crash:


from confluent_kafka.admin import AdminClient
from confluent_kafka.cimpl import NewTopic

client = AdminClient({})
topics = [NewTopic(topic='foo', num_partitions=0)]
client.create_topics(topics)

This pull request, fixes the issue. The problem was that when a topic failed to be created, like in this case where the number of partitions is zero, we would still try to free the topic, causing a null pointer to be de-referenced in rdkafka.

While, this is also a bug in rdkafka, since a null pointer should not be de-referenced either way, we should not pass such a pointer to a library either.

The removed line (`i++;`) caused the most recent topic, which has just
failed to be created, to be passed to rdkafka to be destroyed, causing
a crash.
@ghost
Copy link

ghost commented Nov 26, 2019

It looks like @elektito hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement here.

Once you've signed reply with [clabot:check] to prove it.

Appreciation of efforts,

clabot

@elektito
Copy link
Author

[clabot:check]

@ghost
Copy link

ghost commented Nov 26, 2019

@confluentinc It looks like @elektito just signed our Contributor License Agreement. 👍

Always at your service,

clabot

@edenhill
Copy link
Contributor

Good fix, thank you!

@edenhill edenhill merged commit c20c535 into confluentinc:master Nov 26, 2019
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