Repository navigation
[backport camel-4.22.x] CAMEL-25793: camel-paho-mqtt5 - a failed message with manualAcksEnabled no longer blocks the consumer - #28268
Conversation
…age with manualAcksEnabled no longer blocks the consumer
gnodet-bot
left a comment
There was a problem hiding this comment.
Well-engineered fix for a real MQTT QoS 1 message delivery bug. The rollback() method correctly captures client into a local variable before the null check, which protects against the NPE race with doStop() that nulls the field. The stopClient guard in rollback() is semantically correct despite the field name being a bit misleading. No concerns — the concurrency model is sound.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
davsclaus
left a comment
There was a problem hiding this comment.
Not a straight cherry-pick: PahoMqtt5Consumer.doStart/doStop are adapted because camel-4.22.x does not have CAMEL-24465 (cleanup after a failed start). The adaptation looks right: the reconnect executor is created inside the if (client == null) block, and on 4.22.x a failed start() calls stop(), whose doStop shuts the executor down, so nothing leaks without main's catch block. The test and configuration changes are the same as #28197.
The upgrade note goes into camel-4x-upgrade-guide-4_22.adoc on the branch; the same entry is still needed in the 4_22 guide on main.
Approving, to merge once CI is green.
Claude Code on behalf of @davsclaus.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 13 tested, 25 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
Backport of #28197
Cherry-pick of b7594f0 to camel-4.22.x.
Conflicts resolved automatically by backport bot agent.