Skip to content

Fix delivery report callback: librdkafka passes the message by pointer - #8

Open
stellarthemes wants to merge 1 commit into
BT-OpenSource:mainfrom
stellarthemes:fix-delivery-report-abi
Open

Fix delivery report callback: librdkafka passes the message by pointer#8
stellarthemes wants to merge 1 commit into
BT-OpenSource:mainfrom
stellarthemes:fix-delivery-report-abi

Conversation

@stellarthemes

Copy link
Copy Markdown

librdkafka invokes the delivery report callback as

void dr_msg_cb(rd_kafka_t *rk, const rd_kafka_message_t *rkmessage, void *opaque);

but the binding declares the callback as taking Message by value:

fun conf_set_dr_msg_cb = rd_kafka_conf_set_dr_msg_cb(conf : ConfHandle, cb : (KafkaHandle, Message, Void*) ->)

On x86-64 Linux, a struct this size passed by value is expected in stack memory, so the callback ends up reading memory the caller never wrote — every field in the message is garbage. That's the garbled payload in #2.

On Apple Silicon, a large by-value struct is passed by reference instead, which happens to line up exactly with librdkafka passing a pointer. So the same code looks fine on a Mac — which is why #2 only reproduces on Linux servers.

Changes:

  • the callback now takes Message* and reads fields through .value
  • the payload string is built with the message's len — the payload is not NUL-terminated, so String.new(pointer) could over-read even with the calling convention fixed
  • when err is set, log at error level instead — librdkafka reuses the payload field for the error string in that case

Checked the struct layout against librdkafka 1.8 headers; unit specs pass. Note this changes the callback type, so anyone who registered their own delivery callback with the old signature will need the same one-line change — probably worth a version bump.

Fixes #2.

The dr_msg_cb binding declared the callback as taking Message by value,
but librdkafka invokes it with `const rd_kafka_message_t *`. On x86-64
Linux the by-value declaration makes the callback read stack memory the
caller never wrote, so every field is garbage (issue BT-OpenSource#2's garbled
payload). On Apple Silicon a large by-value struct is passed by
reference, which happens to line up with the pointer librdkafka actually
passes - which is why the bug only shows on Linux.

Also build the payload string with the message length (the payload is
not NUL-terminated) and log the error string when err is set, since
librdkafka reuses the payload field for it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@stufro stufro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT

@stellarthemes

Copy link
Copy Markdown
Author

do i need to do anything to get this merged?

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.

Message delivery log garbled

2 participants