Skip to content

Commit 5d2fda5

Browse files
committed
Fix rd_kafka_last_error() race in headerless produce path
Motivation: The headerless produce path called rd_kafka_produce() then rd_kafka_last_error() to check the result. rd_kafka_last_error() returns a thread-local error code. In Swift Concurrency, multiple Tasks share cooperative pool threads, so a concurrent librdkafka call from another Task can overwrite the error before the first Task reads it. This can cause silent message loss (failed send reads another task's success) or spurious errors. The with-headers path already used rd_kafka_produceva() which returns errors directly via rd_kafka_error_t*, avoiding the race. Modifications: - Remove the if/else branch in produce() that split headerless (rd_kafka_produce) from with-headers (rd_kafka_produceva) - Always use _produceVariadic() -> rd_kafka_produceva() for both paths. The helper already handles empty headers correctly. - Add concurrentSendsDoNotLoseMessages integration test that sends 100 messages from concurrent tasks on a 4-partition topic Result: All produce paths now use rd_kafka_produceva() which returns errors directly, eliminating the thread-local race condition.
1 parent b4f129d commit 5d2fda5

2 files changed

Lines changed: 82 additions & 26 deletions

File tree

Sources/Kafka/RDKafka/RDKafkaClient.swift

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -135,34 +135,18 @@ public final class RDKafkaClient: Sendable {
135135
topic: message.topic
136136
) { topicHandle in
137137
try Self.withMessageKeyAndValueBuffer(for: message) { keyBuffer, valueBuffer in
138-
if message.headers.isEmpty {
139-
// No message headers set, normal produce method can be used.
140-
rd_kafka_produce(
141-
topicHandle,
142-
Int32(message.partition.rawValue),
143-
RD_KAFKA_MSG_F_COPY,
144-
UnsafeMutableRawPointer(mutating: valueBuffer.baseAddress),
145-
valueBuffer.count,
146-
keyBuffer?.baseAddress,
147-
keyBuffer?.count ?? 0,
148-
UnsafeMutableRawPointer(bitPattern: newMessageID)
138+
let errorPointer = try Self.withKafkaCHeaders(for: message.headers) { cHeaders in
139+
try self._produceVariadic(
140+
topicHandle: topicHandle,
141+
partition: Int32(message.partition.rawValue),
142+
messageFlags: RD_KAFKA_MSG_F_COPY,
143+
key: keyBuffer,
144+
value: valueBuffer,
145+
opaque: UnsafeMutableRawPointer(bitPattern: newMessageID),
146+
cHeaders: cHeaders
149147
)
150-
return rd_kafka_last_error()
151-
} else {
152-
let errorPointer = try Self.withKafkaCHeaders(for: message.headers) { cHeaders in
153-
// Setting message headers only works with `rd_kafka_produceva` (variadic arguments).
154-
try self._produceVariadic(
155-
topicHandle: topicHandle,
156-
partition: Int32(message.partition.rawValue),
157-
messageFlags: RD_KAFKA_MSG_F_COPY,
158-
key: keyBuffer,
159-
value: valueBuffer,
160-
opaque: UnsafeMutableRawPointer(bitPattern: newMessageID),
161-
cHeaders: cHeaders
162-
)
163-
}
164-
return rd_kafka_error_code(errorPointer)
165148
}
149+
return rd_kafka_error_code(errorPointer)
166150
}
167151
}
168152

Tests/IntegrationTests/KafkaTests.swift

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,78 @@ func withTestTopic(partitions: Int32 = 1, _ body: (_ testTopic: String) async th
647647
}
648648
}
649649

650+
// MARK: - Concurrent Produce Tests
651+
652+
@Test func concurrentSendsDoNotLoseMessages() async throws {
653+
try await withTestTopic(partitions: 4) { testTopic in
654+
let (producer, events) = try KafkaProducer.makeProducerWithEvents(
655+
config: self.producerConfig,
656+
logger: .kafkaTest
657+
)
658+
659+
let serviceGroupConfiguration = ServiceGroupConfiguration(
660+
services: [producer],
661+
logger: .kafkaTest
662+
)
663+
let serviceGroup = ServiceGroup(configuration: serviceGroupConfiguration)
664+
665+
try await withThrowingTaskGroup(of: Void.self) { group in
666+
group.addTask {
667+
try await serviceGroup.run()
668+
}
669+
670+
// Drain events to collect delivery reports
671+
let acknowledgedCount = ManagedAtomic<Int>(0)
672+
group.addTask {
673+
for await event in events {
674+
switch event {
675+
case .deliveryReports(let reports):
676+
for report in reports {
677+
if case .acknowledged = report.status {
678+
acknowledgedCount.wrappingIncrement(ordering: .relaxed)
679+
}
680+
}
681+
default:
682+
break
683+
}
684+
}
685+
}
686+
687+
// Send 100 messages concurrently from multiple tasks — exercises the
688+
// rd_kafka_produceva path under contention. Before the fix, the old
689+
// rd_kafka_produce + rd_kafka_last_error() path could return wrong
690+
// error codes when tasks shared a cooperative thread pool thread.
691+
let messageCount = 100
692+
try await withThrowingTaskGroup(of: Void.self) { sendGroup in
693+
for i in 0..<messageCount {
694+
sendGroup.addTask {
695+
let message = KafkaProducerMessage(
696+
topic: testTopic,
697+
key: "key-\(i)",
698+
value: "value-\(i)"
699+
)
700+
try producer.send(message)
701+
}
702+
}
703+
try await sendGroup.waitForAll()
704+
}
705+
706+
// Wait for delivery reports
707+
for _ in 0..<100 {
708+
if acknowledgedCount.load(ordering: .relaxed) >= messageCount { break }
709+
try await Task.sleep(for: .milliseconds(100))
710+
}
711+
712+
#expect(
713+
acknowledgedCount.load(ordering: .relaxed) >= messageCount,
714+
"Expected \(messageCount) acknowledged, got \(acknowledgedCount.load(ordering: .relaxed))"
715+
)
716+
717+
await serviceGroup.triggerGracefulShutdown()
718+
}
719+
}
720+
}
721+
650722
// MARK: - storeOffset Tests
651723

652724
@Test func produceAndConsumeWithStoreOffset() async throws {

0 commit comments

Comments
 (0)