Fix datarace in rd_kafka_broker_fetch_toppars #5266
Conversation
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a data race condition in the rd_kafka_broker_fetch_toppars function by adding proper locking around access to the rktp_leader_epoch field. The race condition was occurring when multiple threads could simultaneously access and check the leader epoch value without synchronization.
Key Changes:
- Added
rd_kafka_toppar_lock()andrd_kafka_toppar_unlock()calls to protect concurrent access torktp->rktp_leader_epoch
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rd_kafka_toppar_lock(rktp); | ||
| if (rktp->rktp_leader_epoch < 0 && | ||
| rd_kafka_has_reliable_leader_epochs(rkb)) { | ||
| /* If current leader epoch is set to -1 and |
There was a problem hiding this comment.
The lock is released before writing to the buffer at line 1128 (FetchOffset), which may also access rktp fields. Verify that the fetch offset write at line 1128 doesn't require the same lock protection, or if it does, the unlock should be moved after that operation.
| rd_kafka_buf_write_i32(rkbuf, | ||
| rktp->rktp_leader_epoch); | ||
| } | ||
| rd_kafka_toppar_unlock(rktp); |
There was a problem hiding this comment.
The lock is released before writing to the buffer at line 1128 (FetchOffset), which may also access rktp fields. Verify that the fetch offset write at line 1128 doesn't require the same lock protection, or if it does, the unlock should be moved after that operation.
1 participant
Fixes #5262