Skip to content

Add missing wrlock around rd_kafka_metadata_cache_hint call#5055

Closed
Marcin Krystianc (marcin-krystianc) wants to merge 1 commit intoconfluentinc:masterfrom
marcin-krystianc:dev-20250428-wrlock
Closed

Add missing wrlock around rd_kafka_metadata_cache_hint call#5055
Marcin Krystianc (marcin-krystianc) wants to merge 1 commit intoconfluentinc:masterfrom
marcin-krystianc:dev-20250428-wrlock

Conversation

@marcin-krystianc
Copy link
Copy Markdown
Contributor

Fixes #5051

This PR addresses a heap-use-after-free error detected by AddressSanitizer within the rd_kafka_metadata_cache_topics_to_list function. The issue was particularly reproducible during tests involving frequent topic deletion and recreation, which heavily exercises the metadata cache logic.

The root cause was identified as missing write lock protection when invoking the rd_kafka_metadata_cache_hint. Without proper locking, concurrent threads (e.g., the broker communication thread handling metadata updates and potentially other threads accessing metadata) could race to modify or read the hint, leading to metadata cache corruption. This corruption could manifest as accessing freed memory when iterating through the cache, resulting in the observed heap-use-after-free.

With this change, the previously reported ASan error is resolved when running the test case involving rapid topic recreation.

@confluent-cla-assistant
Copy link
Copy Markdown

🎉 All Contributor License Agreements have been signed. Ready to merge.
✅ marcin-krystianc
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

@emasab
Copy link
Copy Markdown
Contributor

Thanks for the fix Marcin Krystianc (@marcin-krystianc) ! Could you rebase/update the PR?

@marcin-krystianc
Copy link
Copy Markdown
Contributor Author

Thanks for the fix Marcin Krystianc (@marcin-krystianc) ! Could you rebase/update the PR?

I've rebased it, feel free to push any other necessary changes or rebase again if needed.

Marcin Krystianc (marcin-krystianc) added a commit to marcin-krystianc/librdkafka that referenced this pull request Apr 29, 2025
 - confluentinc#4972 (Avoid unnecessary producer epoch bumps)
 - confluentinc#4989 (Fully utilize the max.in.flight.requests.per.connection parameter on the idempotent producer)
 - confluentinc#5055 (Add missing wrlock around rd_kafka_metadata_cache_hint call)
Marcin Krystianc (marcin-krystianc) added a commit to marcin-krystianc/librdkafka that referenced this pull request Apr 29, 2025
 - confluentinc#4972 (Avoid unnecessary producer epoch bumps)
 - confluentinc#4989 (Fully utilize the max.in.flight.requests.per.connection parameter on the idempotent producer)
 - confluentinc#5055 (Add missing wrlock around rd_kafka_metadata_cache_hint call)
Marcin Krystianc (marcin-krystianc) added a commit to marcin-krystianc/librdkafka that referenced this pull request Apr 29, 2025
 - confluentinc#4972 (Avoid unnecessary producer epoch bumps)
 - confluentinc#4989 (Fully utilize the max.in.flight.requests.per.connection parameter on the idempotent producer)
 - confluentinc#5055 (Add missing wrlock around rd_kafka_metadata_cache_hint call)
Marcin Krystianc (marcin-krystianc) added a commit to G-Research/librdkafka that referenced this pull request Apr 30, 2025
- confluentinc#4972 (Avoid unnecessary producer epoch bumps)
 - confluentinc#4989 (Fully utilize the max.in.flight.requests.per.connection parameter on the idempotent producer)
 - confluentinc#5055 (Add missing wrlock around rd_kafka_metadata_cache_hint call)
@emasab
Copy link
Copy Markdown
Contributor

moved to the internal branch in #5066
Thanks Marcin Krystianc (@marcin-krystianc)

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.

ASan: heap-use-after-free in rd_kafka_metadata_cache_topics_to_list

2 participants