[
https://issues.apache.org/jira/browse/CAMEL-25855?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
shashank updated CAMEL-25855:
-----------------------------
Description:
For an InOut exchange, {{QuickfixjProducer.sendMessage}} registers a
correlation rule (the exchange and its {{CorrelationCriteria}}) in the
{{MessageCorrelator}} of the engine before it sends the FIX message, then waits
for the reply up to {{CorrelationTimeout}} (default 1000 ms).
{{MessageCorrelator}} removes a rule only in {{onEvent}}, when a message that
matches its criteria arrives:
{code:java}
for (MessageCorrelationRule rule : rules) {
if (rule.getMessageCriteria().evaluate(message)) {
rule.setReplyMessage(message);
rules.remove(rule);
rule.getLatch().countDown();
}
}
{code}
So a rule stays registered for good, holding its exchange, when:
* the wait times out and no matching message ever arrives (the counterparty
answers with a session level Reject or a BusinessMessageReject that the
criteria do not match, or does not answer at all);
* the send fails: {{Session.send}} returns false (session not logged on, the
producer throws {{CannotSendException}}) or throws. The exchange fails, so
nobody waits for a reply (QuickFIX/J has stored an application message before
it found the session not logged on, and may still resend it after the next
logon).
The rules are kept in a {{CopyOnWriteArrayList}} of the engine, which lives as
long as the component, so they add up for every such request (a route that
retries InOut sends while the session is down adds one per attempt). Each add
copies the whole list, and every FIX message the engine receives or sends
(application and admin messages, heartbeats) is evaluated against every leaked
rule on the session thread.
*Fix*: the waiting {{Callable}} removes its rule in a finally block (reply,
timeout or interrupt), and the producer removes the rule of an exchange whose
send failed (new {{MessageCorrelator.cancelReply(Exchange)}}). The rule is
still registered before the send, so a fast reply is not missed.
*Tests*: new {{QuickfixjProducerCorrelationCleanupTest}} (3 tests, same mocked
session as {{QuickfixjProducerTest}}). On main:
{{timedOutRequestsDoNotStayRegistered}} (50 InOut requests with a 10 ms timeout
and no reply) fails with {{correlation rules left after 50 timed out requests
==> expected: <0> but was: <50>}};
{{requestThatCannotBeSentDoesNotStayRegistered}} and
{{requestWhoseSendThrowsDoesNotStayRegistered}} fail with {{expected: <0> but
was: <1>}}. All pass with the fix. camel-quickfix unit tests with the fix: 46
run, 0 failures (including the existing InOut tests of
{{QuickfixjProducerTest}}).
*How it was found*: a review of request/reply correlation in producers (where a
waiting request is registered and where it is removed again). The JUnit tests
above are the evidence; it was not run against a FIX counterparty.
*Affected*: main, camel-4.23.x, camel-4.22.x, camel-4.18.x and camel-4.14.x
(same {{MessageCorrelator}} and {{QuickfixjProducer}}; read, not run).
_Filed with Claude Code on behalf of allthingssecurity._
was:
For an InOut exchange, {{QuickfixjProducer.sendMessage}} registers a
correlation rule (the exchange and its {{CorrelationCriteria}}) in the
{{MessageCorrelator}} of the engine before it sends the FIX message, then waits
for the reply up to {{CorrelationTimeout}} (default 1000 ms).
{{MessageCorrelator}} removes a rule only in {{onEvent}}, when a message that
matches its criteria arrives:
{code:java}
for (MessageCorrelationRule rule : rules) {
if (rule.getMessageCriteria().evaluate(message)) {
rule.setReplyMessage(message);
rules.remove(rule);
rule.getLatch().countDown();
}
}
{code}
So a rule stays registered for good, holding its exchange, when:
* the wait times out and no matching message ever arrives (the counterparty
answers with a session level Reject or a BusinessMessageReject that the
criteria do not match, or does not answer at all);
* the message is not sent: {{Session.send}} returns false (session not logged
on, the producer throws {{CannotSendException}}) or throws.
The rules are kept in a {{CopyOnWriteArrayList}} of the engine, which lives as
long as the component, so they add up for every such request (a route that
retries InOut sends while the session is down adds one per attempt). Each add
copies the whole list, and every FIX message the engine receives or sends
(application and admin messages, heartbeats) is evaluated against every leaked
rule on the session thread.
*Fix*: the waiting {{Callable}} removes its rule in a finally block (reply,
timeout or interrupt), and the producer removes the rule of an exchange whose
message was not sent (new {{MessageCorrelator.cancelReply(Exchange)}}). The
rule is still registered before the send, so a fast reply is not missed.
*Tests*: new {{QuickfixjProducerCorrelationCleanupTest}} (3 tests, same mocked
session as {{QuickfixjProducerTest}}). On main:
{{timedOutRequestsDoNotStayRegistered}} (50 InOut requests with a 10 ms timeout
and no reply) fails with {{correlation rules left after 50 timed out requests
==> expected: <0> but was: <50>}};
{{requestThatCannotBeSentDoesNotStayRegistered}} and
{{requestWhoseSendThrowsDoesNotStayRegistered}} fail with {{expected: <0> but
was: <1>}}. All pass with the fix. camel-quickfix unit tests with the fix: 46
run, 0 failures (including the existing InOut tests of
{{QuickfixjProducerTest}}).
*How it was found*: a review of request/reply correlation in producers (where a
waiting request is registered and where it is removed again). The JUnit tests
above are the evidence; it was not run against a FIX counterparty.
*Affected*: main, camel-4.23.x, camel-4.22.x, camel-4.18.x and camel-4.14.x
(same {{MessageCorrelator}} and {{QuickfixjProducer}}; read, not run).
_Filed with Claude Code on behalf of allthingssecurity._
> camel-quickfix - an InOut request that times out or cannot be sent leaves its
> correlation rule and exchange in the engine for good
> ----------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-25855
> URL: https://issues.apache.org/jira/browse/CAMEL-25855
> Project: Camel
> Issue Type: Bug
> Components: camel-quickfix
> Reporter: shashank
> Assignee: shashank
> Priority: Major
>
> For an InOut exchange, {{QuickfixjProducer.sendMessage}} registers a
> correlation rule (the exchange and its {{CorrelationCriteria}}) in the
> {{MessageCorrelator}} of the engine before it sends the FIX message, then
> waits for the reply up to {{CorrelationTimeout}} (default 1000 ms).
> {{MessageCorrelator}} removes a rule only in {{onEvent}}, when a message that
> matches its criteria arrives:
> {code:java}
> for (MessageCorrelationRule rule : rules) {
> if (rule.getMessageCriteria().evaluate(message)) {
> rule.setReplyMessage(message);
> rules.remove(rule);
> rule.getLatch().countDown();
> }
> }
> {code}
> So a rule stays registered for good, holding its exchange, when:
> * the wait times out and no matching message ever arrives (the counterparty
> answers with a session level Reject or a BusinessMessageReject that the
> criteria do not match, or does not answer at all);
> * the send fails: {{Session.send}} returns false (session not logged on, the
> producer throws {{CannotSendException}}) or throws. The exchange fails, so
> nobody waits for a reply (QuickFIX/J has stored an application message before
> it found the session not logged on, and may still resend it after the next
> logon).
> The rules are kept in a {{CopyOnWriteArrayList}} of the engine, which lives
> as long as the component, so they add up for every such request (a route that
> retries InOut sends while the session is down adds one per attempt). Each add
> copies the whole list, and every FIX message the engine receives or sends
> (application and admin messages, heartbeats) is evaluated against every
> leaked rule on the session thread.
> *Fix*: the waiting {{Callable}} removes its rule in a finally block (reply,
> timeout or interrupt), and the producer removes the rule of an exchange whose
> send failed (new {{MessageCorrelator.cancelReply(Exchange)}}). The rule is
> still registered before the send, so a fast reply is not missed.
> *Tests*: new {{QuickfixjProducerCorrelationCleanupTest}} (3 tests, same
> mocked session as {{QuickfixjProducerTest}}). On main:
> {{timedOutRequestsDoNotStayRegistered}} (50 InOut requests with a 10 ms
> timeout and no reply) fails with {{correlation rules left after 50 timed out
> requests ==> expected: <0> but was: <50>}};
> {{requestThatCannotBeSentDoesNotStayRegistered}} and
> {{requestWhoseSendThrowsDoesNotStayRegistered}} fail with {{expected: <0> but
> was: <1>}}. All pass with the fix. camel-quickfix unit tests with the fix: 46
> run, 0 failures (including the existing InOut tests of
> {{QuickfixjProducerTest}}).
> *How it was found*: a review of request/reply correlation in producers (where
> a waiting request is registered and where it is removed again). The JUnit
> tests above are the evidence; it was not run against a FIX counterparty.
> *Affected*: main, camel-4.23.x, camel-4.22.x, camel-4.18.x and camel-4.14.x
> (same {{MessageCorrelator}} and {{QuickfixjProducer}}; read, not run).
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
