Fix Producer.purge() ignoring boolean flags on big-endian platforms - #2345
Fix Producer.purge() ignoring boolean flags on big-endian platforms#2345Devarsh Patel (Devarsh010) wants to merge 1 commit into
Conversation
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
There was a problem hiding this comment.
🟡 Changes recommended
The blocking flag appears to be applied with inverted semantics (blocking=True sets RD_KAFKA_PURGE_F_NON_BLOCKING), which contradicts the documented behavior and should be corrected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes Producer.purge() boolean keyword argument parsing so in_queue, in_flight, and blocking can be reliably set to False on big-endian platforms by switching from the 1-byte "b" parser format to the boolean-predicate "p" format.
Changes:
- Update
Producer.purge()argument parsing to use"|ppp"and document the endianness issue in-code. - Add a changelog entry describing the big-endian behavior fix.
File summaries
| File | Description |
|---|---|
src/confluent_kafka/src/Producer.c |
Switches purge() kwarg parsing to "p" booleans and adds rationale comment. |
CHANGELOG.md |
Documents the Producer.purge() big-endian boolean parsing fix. |
Review details
Suppressed comments (1)
src/confluent_kafka/src/Producer.c:1043
- The
blockingargument’s semantics appear inverted: the docstring saysblocking=Falseshould not wait, but the code setsRD_KAFKA_PURGE_F_NON_BLOCKINGwhenblockingis true. With the bool parsing fixed, this mismatch is now reliably observable on all platforms.
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ppp", kws, &in_queue,
&in_flight, &blocking))
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - Use `asyncio.get_running_loop()` instead of `asyncio.get_event_loop()` to avoid creating a new event loop and raise an error in case a loop isn't available (@AlexCai26, #2339). | ||
| - Fix `Producer.purge()` ignoring its `in_queue`, `in_flight` and `blocking` | ||
| arguments on big-endian platforms (e.g. s390x). They were parsed with the | ||
| 1-byte `"b"` format into 4-byte `int` targets, so on big-endian the value |
| /* Use "p" (bool predicate -> int), not "b" (one byte): the targets are | ||
| * 4-byte ints, so "b" stores a single byte, which lands on the low byte | ||
| * on little-endian but the high byte on big-endian. There the flags, | ||
| * pre-initialised to 1, could never be cleared, so e.g. in_queue=False | ||
| * was ignored and the queue was purged anyway. */ |
edc03c6 to
370a831
Compare
|
Producer.purge() parsed its in_queue, in_flight and blocking arguments with PyArg_ParseTupleAndKeywords's "|bbb" format , which is the 1-byte "b" (unsigned char) format into 4-byte int targets pre-initialised to 1. "b" writes a single byte, which on little-endian lands on the low byte (so it happens to work) but on big-endian lands on the high byte, leaving the int as 0x00000001. The flags can therefore never be set to False on big-endian, so purge(in_queue=False) purges the queue anyway.
Fixed by using the "p" (boolean predicate → int) format, which is the correct format for these bool-typed arguments and produces identical results on little-endian. This is the only "b"-format PyArg call in the C sources.
The existing tests/test_Producer.py::test_purge already covers this: it asserts purge(in_queue=False) does not purge, which fails deterministically on big-endian before this change and passes after. Validated on a native s390x host (built against librdkafka.redist 2.15.0): before the fix test_purge fails, after it passes; little-endian behaviour is unchanged.