allthingssecurity opened a new pull request, #27698: URL: https://github.com/apache/camel/pull/27698
# Description [CAMEL-25559](https://issues.apache.org/jira/browse/CAMEL-25559) With the pooled exchange factory (`camel.main.exchangeFactory=pooled`, not the default, and deprecated in 4.23), a consumer that gives back an exchange it did not route puts it into the pool twice. The next two messages of that consumer then get the same exchange object. The mechanism, in `PooledExchangeFactory`: - an exchange created with auto release (`createExchange(true)`) has an on-done task that calls `release(exchange)`; - `release` calls `done()` on the exchange and then offers it to the pool; - on the normal path the unit of work calls `done()`, the task calls `release`, the `done()` inside `release` does nothing (the exchange is already done), and the exchange is offered once; - when the consumer calls `releaseExchange(exchange, true)` for an exchange that is not done, `release` calls `done()`, which resets the exchange and runs the task, which calls `release` again. The nested call offers the exchange, and the outer call offers it again. The same double insert happens with `releaseExchange(exchange, false)` for an exchange created with auto release (`DefaultConsumer.releaseExchange` calls `done()`, the task releases it, and then `release` offers it again), and when an exchange that the unit of work already gave back is released once more. The pool is an `ArrayBlockingQueue`, which accepts the same object twice. Consumers on main that give back exchanges they did not route with `releaseExchange(exchange, true)`: `GenericFileConsumer` (every file whose `processStrategy.begin` fails or returns false, for example when the read lock was not acquired: `processBatch` collects those exchanges and gives them back at the end of the batch; the files beyond `maxMessagesPerPoll` with `eagerMaxMessagesPerPoll=false`; and the rest of a batch when the consumer is stopping), `MailConsumer`, `AWS2S3Consumer`, `IBMCOSConsumer`, `BlobConsumer`, `OBSConsumer`, `OSSConsumer`. Consumers that create with auto release and give back with `false` include `GoogleCloudStorageConsumer` and `GoogleFirestoreConsumer`. So for the file based components no special option is needed: a file route with default options and the pooled factory is enough, as soon as two files of one poll cannot be locked. I first saw it while working on CAMEL-25558 (camel-aws2-s3), where two objects of one poll shared an exchange. The change is in `PooledExchangeFactory.release` only, so every caller is covered and no consumer has to change: - 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 an auto-release exchange makes sure the exchange is done (which runs the task if it was not) and does not offer it itself; - an exchange without auto release is handled as before: `done()`, then offered. Cost on the normal path: less than before, since the task no longer calls `release` and with it `done()` a second time; a direct `release` call reads `isAutoRelease()` once. No new state, no lock. Statistics: each exchange is counted once as released (or discarded when the pool is full). No signature changes. One detail: for an auto-release exchange `release` now returns `true` unless the reset failed, also when the on-done task had to discard the exchange because the pool was full. The javadoc of the method says so. Nothing in the repository reads that return value. `PooledProcessorExchangeFactory` (the copies made by Split, Multicast, Wire Tap and others) never registers an on-done task, so its `release` offers once. It is not changed. No upgrade-guide note: the only difference is that the pool no longer holds an exchange twice. Tests, in camel-core: - `PooledExchangeFactoryReleaseNotRoutedTest` (4 tests) takes the consumer of a `direct:` route with `PooledExchangeFactory`, creates an exchange, gives it back, and checks the pool size (1), the statistics (created 1, released 1, discarded 0) and that the next two exchanges are different objects. Without the change three of them fail with `exchanges in the pool ==> expected: <1> but was: <2>`: created with auto release and given back with `true`; given back with `false`; routed first and then given back again. The fourth (created and given back without auto release) passes without the change too; it covers the other branch of `release`. - `FileConsumeNotEagerMaxMessagesPerPollPooledExchangeTest`: four files, `maxMessagesPerPoll=2&eagerMaxMessagesPerPoll=false&sortBy=file:name`, pooled exchanges; the route must receive `AAA`, `BBB`, `CCC`, `DDD`. Without the change: `Body of message: 2. Expected: <CCC> but was: <DDD>`. - `FileConsumeReadLockNotAcquiredPooledExchangeTest`: two files, 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 route must receive `AAA`, `BBB`. Without the change: `Received message count. Expected: <2> but was: <1>`. - The full camel-core suite with the change (`mvn -o -pl core/camel-core -am test -Dmaven.test.failure.ignore=true`, so that camel-core runs against the changed camel-base-engine from the reactor; nothing was installed): 8164 tests, 0 failures, 1 error, 45 skipped, 2 flakes, 4 min 12 s for the module. It includes the three new classes (6 tests) and the existing pooled exchange and file consumer tests. The tests of the upstream modules built by `-am` ran as well and passed. - The error is `ValidatorExternalResourceTest`: it loads an XSD from `raw.githubusercontent.com` and got `SocketTimeoutException: Connect timed out` on my machine in all three attempts. It fails the same way on main without the change (run of that class on a clean checkout of main, same three attempts). - The flakes are `FileConsumerInterceptEmptyFileTest.testExcludeZeroLengthFiles` and `FileExclusiveReadNoneStrategyTest.testPollFileWhileSlowFileIsBeingWritten`: each failed once and passed on the automatic re-run. Neither uses the pooled exchange factory. On a clean checkout of main both passed in one run, so I cannot show that they are flaky on main as well; I only saw them fail once here, under the load of the full suite. - Run before the last revision of this commit (which added the javadoc on `release`, the read lock test, and removed an annotation from a test; the code of `PooledExchangeFactory` is otherwise the same): `ManagedPooledExchangeTest` of camel-management (1), and the pooled exchange tests of some components, built against the changed camel-base-engine: `NettyTextlineInOnlyPooledExchangeTest` (2), `JmsInOnlyPooledExchangeTest` (2), `InOnlyPooledExchangeTest` of camel-sjms (2), `SqlConsumerRollbackOnlyPooledExchangeTest` (3), `MyBatisOnConsumeFailedPooledExchangeTest` (3), `JooqConsumerDeleteFailedPooledExchangeTest` (3): all pass. Not run: the full suites of camel-management and of the components listed above (of those I ran the classes named here), and the tests on the 4.22.x, 4.18.x and 4.14.x branches. camel-base-engine has no tests of its own; camel-file has its tests in camel-core. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [ ] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). (The issue is drafted and not filed yet: `CAMEL-25559` above is a placeholder.) # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built camel-core, camel-management and the components named above with their upstream modules, and ran the tests listed, including the full camel-core suite. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
