Andrea Cosentino created CAMEL-25308:
----------------------------------------
Summary: camel-smpp: the deliverSm and dataSm paths create
exchanges on the endpoint instead of the consumer, so pooling never applies and
fromRouteId is unset
Key: CAMEL-25308
URL: https://issues.apache.org/jira/browse/CAMEL-25308
Project: Camel
Issue Type: Bug
Components: camel-smpp
Reporter: Andrea Cosentino
Assignee: Andrea Cosentino
{{Endpoint.createExchange}} documents the rule explicitly:
{quote}*Important:* Consumers should use {{Consumer#createExchange(boolean)}}
to create an exchange for which the consumer received a message.{quote}
{{MessageReceiverListenerImpl}} follows that rule in one of its three inbound
paths and not in the other two.
h2. The three paths disagree
{{onAcceptAlertNotification}} is correct - it builds the exchange through the
consumer and releases it afterwards:
{code:java}
public Exchange createOnAcceptAlertNotificationExchange(AlertNotification
alertNotification) {
Exchange exchange = consumer.createExchange(false);
...
}
...
consumer.releaseExchange(exchange, false);
{code}
{{onAcceptDeliverSm}} and {{onAcceptDataSm}} instead go through the endpoint:
{code:java}
exchange = endpoint.createOnAcceptDeliverSmExchange(deliverSm);
...
Exchange exchange = endpoint.createOnAcceptDataSm(dataSm,
newMessageId.getValue());
{code}
and both of those end in {{DefaultEndpoint.createExchange(pattern)}}, which is
{{DefaultExchange.newFromEndpoint(this, pattern)}} - a plain exchange that
never goes near the configured {{ExchangeFactory}}. Neither path releases the
exchange either.
h2. What it costs
This is *not* a pool leak: because nothing is taken from the pool, nothing is
lost from it. What is lost is:
* *Exchange pooling never applies to received messages.*
{{camel.main.exchange-factory=pooled}} has no effect on {{deliverSm}}, which is
the path every received SMS takes, nor on {{dataSm}} - only on the
comparatively rare {{alertNotification}}.
* *{{fromRouteId}} is unset on every received message.*
{{DefaultConsumer.createExchange}} does
{{answer.getExchangeExtension().setFromRouteId(routeId)}}; the endpoint path
does not, so anything reading {{Exchange.getFromRouteId()}} - MDC logging,
tracing, management - sees null for inbound SMPP traffic.
h2. Suggested fix
Build both exchanges in {{MessageReceiverListenerImpl}} through
{{consumer.createExchange(false)}}, the way the alert-notification path already
does, carrying over the endpoint's {{ExchangePattern}} and the binding, and
release them in a {{finally}} so the {{ProcessRequestException}} path releases
too.
The endpoint's {{createOnAcceptDeliverSmExchange}} and {{createOnAcceptDataSm}}
are public, so they are left in place rather than removed.
Found while auditing camel-smpp; the component had no behavioural Jira since
2023.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
