[jira] [Resolved] (CAMEL-25559) camel-core - with the pooled exchange factory, an exchange that a consumer gives back without routing it is put into the pool twice, and the next two messages share one exchange

["Claus Ibsen (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/CAMEL-25559?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Claus Ibsen resolved CAMEL-25559.
---------------------------------
    Resolution: Fixed

Fixed by https://github.com/apache/camel/pull/27698 (merged to main as 28e150782cdc, 4.24.0).

_Claude Code on behalf of Claus Ibsen_

> camel-core - with the pooled exchange factory, an exchange that a consumer gives back without routing it is put into the pool twice, and the next two messages share one exchange
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-25559
>                 URL: https://issues.apache.org/jira/browse/CAMEL-25559
>             Project: Camel
>          Issue Type: Bug
>          Components: camel-core
>            Reporter: shashank
>            Assignee: shashank
>            Priority: Minor
>             Fix For: 4.24.0
>
>
> This needs the pooled exchange factory ({{camel.main.exchangeFactory=pooled}}). It is opt-in (not the default), and exchange pooling is deprecated since 4.23 (CAMEL-24979), hence the priority Minor (as CAMEL-24800, the previous pooled exchange fix). A fix is still useful: the factory is still shipped and can be selected on main, it is not deprecated on the 4.22.x, 4.18.x and 4.14.x branches, the effect is silent (a message is routed with the content of another one), and the change is limited to one method.
> {{PooledExchangeFactory.createPooledExchange}} registers an on-done task on every exchange created with auto release (line 136 at main 3a6cdba883a5); the task calls {{release(exchange)}} (lines 159-165). {{release}} (lines 100-124) calls {{done()}} on the exchange and then {{pool.offer(exchange)}}. {{DefaultPooledExchange.done()}} (lines 81-122) resets the exchange and runs the task, but only when the exchange is not done yet.
> * Normal path: the unit of work calls {{done()}} ({{DefaultUnitOfWork}} line 299), the task calls {{release}}, the {{done()}} inside {{release}} does nothing, one offer.
> * A consumer gives back an exchange it did not route, {{DefaultConsumer.releaseExchange(exchange, true)}} (lines 160-168): {{release}} -> {{done()}} -> the task -> {{release}} again -> {{done()}} does nothing -> offer; back in the outer {{release}} -> second offer.
> * {{releaseExchange(exchange, false)}} for an exchange created with auto release: {{DefaultConsumer}} calls {{done()}} (line 164) -> the task -> {{release}} -> offer; then {{exchangeFactory.release}} (line 166) -> second offer.
> * An exchange that was routed (the unit of work already gave it back) and is then released again: second offer.
> The pool is an {{ArrayBlockingQueue}} ({{PooledObjectFactorySupport}} line 51) and accepts the same object twice. The next two {{createExchange}} calls of that consumer return the same exchange, so two messages share the body, headers and properties, and the exchange is reset when the first of them completes.
> Consumers on main that give back exchanges they did not route:
> * with {{releaseExchange(exchange, true)}}: {{GenericFileConsumer}} line 295 (file, ftp, sftp and the other file based components: every file whose {{processStrategy.begin}} fails or returns false, for example when the read lock was not acquired ({{processExchange}} lines 430-458 returns false, {{processBatch}} collects the exchange at line 264 and gives it back at line 269); the files beyond {{maxMessagesPerPoll}} with {{eagerMaxMessagesPerPoll=false}}; and the rest of a batch when the consumer is stopping). The first of these needs no special option: a file route with default options and the pooled factory is enough, as soon as two files of one poll cannot be locked; {{MailConsumer}} line 422, {{AWS2S3Consumer}} lines 313 and 398, {{IBMCOSConsumer}} line 166, {{BlobConsumer}} line 184, {{OBSConsumer}} line 283, {{OSSConsumer}} line 217;
> * created with auto release and given back with {{false}}: for example {{GoogleCloudStorageConsumer}} line 221 and {{GoogleFirestoreConsumer}} line 89.
> h3. Reproduction
> New tests in camel-core (three classes).
> {{PooledExchangeFactoryReleaseNotRoutedTest}}: the consumer of a {{direct:}} route with {{PooledExchangeFactory}} and statistics; create an exchange, give it back, then check the pool size (1), created 1 / released 1 / discarded 0, and that the next two exchanges are different objects. On main three of the four tests fail with {{exchanges in the pool ==> expected: <1> but was: <2>}}:
> * {{autoReleaseExchangeThatWasNotRoutedIsPooledOnce}} ({{createExchange(true)}}, {{releaseExchange(exchange, true)}});
> * {{autoReleaseExchangeThatWasNotRoutedIsPooledOnceWhenReleasedAsNotAutoRelease}} ({{releaseExchange(exchange, false)}});
> * {{autoReleaseExchangeThatWasRoutedIsPooledOnceWhenReleasedAgain}}.
> {{exchangeWithoutAutoReleaseIsPooledOnce}} passes on main; it covers the other branch.
> {{FileConsumeNotEagerMaxMessagesPerPollPooledExchangeTest}}: files {{a.txt}} to {{d.txt}} with bodies AAA to DDD, {{from("file:...?maxMessagesPerPoll=2&eagerMaxMessagesPerPoll=false&sortBy=file:name")}} to a mock that expects AAA, BBB, CCC, DDD. On main: {{mock://result Body of message: 2. Expected: <CCC> but was: <DDD>}}. The first poll gives back the exchanges of {{c.txt}} and {{d.txt}}, each twice; in the second poll both files get the same exchange, and the route receives the content of {{d.txt}} for the third message.
> {{FileConsumeReadLockNotAcquiredPooledExchangeTest}}: files {{a.txt}} (AAA) and {{b.txt}} (BBB), a file route with default options apart from {{sortBy=file:name}} and an {{exclusiveReadLockStrategy}} that refuses the lock for the two files of the first poll; the mock expects AAA, BBB. On main: {{mock://result Received message count. Expected: <2> but was: <1>}}. The first poll gives back both exchanges, each twice, and in the second poll both files get the same exchange.
> h3. Proposed fix
> In {{PooledExchangeFactory.release}} only, so that all callers are covered: an auto-release exchange is put into the pool by its on-done task and only by it (the task now offers the exchange directly instead of calling {{release}}); {{release}} for such an exchange makes sure it is done and does not offer it itself. An exchange without auto release is handled as before. No signature changes, no new state; the normal path does one {{done()}} call less. Each exchange is counted once in the statistics. For an auto-release exchange {{release}} returns {{true}} unless the reset failed, also when the pool was full and the exchange was discarded (now said in the javadoc of the method); nothing in the repository reads that value.
> {{PooledProcessorExchangeFactory}} has no on-done task and offers once; not changed.
> No upgrade-guide note: nothing changes except that the pool no longer holds an exchange twice.
> With the fix, the full camel-core suite: 8164 tests, 0 failures, 1 error, 45 skipped, 2 flakes. The error is {{ValidatorExternalResourceTest}}, which loads an XSD from raw.githubusercontent.com and timed out on the connection; it fails the same way on main without the change. The two flakes ({{FileConsumerInterceptEmptyFileTest}}, {{FileExclusiveReadNoneStrategyTest}}) failed once and passed on the automatic re-run; neither uses the pooled factory. {{ManagedPooledExchangeTest}} (1) and the pooled exchange tests of camel-netty, camel-jms, camel-sjms, camel-sql, camel-mybatis and camel-jooq (15 tests) pass.
> Found by reading the release path after two objects of one poll shared an exchange in a camel-aws2-s3 test (CAMEL-25558), then confirmed with the tests above.
> Affected: main. {{PooledExchangeFactory}} on camel-4.22.x (3daa566cad1c), camel-4.18.x (4d0578ee450b) and camel-4.14.x (a01a1e1e913e) differs from main only by the three lines of the deprecation (javadoc tag and annotation), and the main-code part of the patch applies to each of the three branches as it is ({{git apply --check}}, all four hunks at an offset of -3 lines). {{DefaultPooledExchange.done()}} runs the on-done task there too. The patch was not built and the tests were not run on those branches. {{release}} has this form since CAMEL-18210 (3.14.4, 3.18.0).
> Duplicate check (2026-10-11): JIRA text "PooledExchangeFactory" (7: CAMEL-25558, CAMEL-25527, CAMEL-24979, CAMEL-24933, CAMEL-24800, CAMEL-24133, CAMEL-23686; none about a double insert on release), "releaseExchange" (5, none about this), "exchange pool twice" (17; closest is CAMEL-18210 from 2022, resolved, which fixed the unit of work calling done for exchanges without auto release), "eagerMaxMessagesPerPoll pooled" (none); GitHub pull requests "PooledExchangeFactory", "pooled exchange release" and open pull requests "releaseExchange": none for this (the only open hit for "PooledExchangeFactory" is #24844, a new component).
> _Filed with Claude Code on behalf of allthingssecurity._



--
This message was sent by Atlassian Jira
(v8.20.10#820010)