[
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)
