allthingssecurity opened a new pull request, #28537: URL: https://github.com/apache/camel/pull/28537
# Description [CAMEL-26050](https://issues.apache.org/jira/browse/CAMEL-26050) The thrift producer is asynchronous by default (`synchronous=false`). In `doStart` it builds its client with `ThriftUtils.constructAsyncClientInstance`, which creates a `new TAsyncClientManager()` each time: a `TAsyncClientManager#SelectorThread` and an NIO selector. `ThriftProducer.doStop` only closes the transport, and nothing ever calls `TAsyncClientManager.stop()`. A route restart creates a new producer, so each restart (and each producer evicted from a `toD` or `ProducerTemplate` cache) leaks one selector thread and one selector, until the JVM exits. The module had no test of the asynchronous producer (all producer tests use `synchronous=true`). Changes: - `ThriftProducer` creates the `TAsyncClientManager` itself, passes it to a new `ThriftUtils.constructAsyncClientInstance(..., TAsyncClientManager)` overload, and stops it in `doStop`, after the transport is closed. In libthrift 0.25.0 `stop()` ends the selector thread, which closes the selector (read with javap). If the start fails after the manager was created, `BaseService.start` calls `stop()`, so the manager is stopped then too. - The existing 5-argument `ThriftUtils.constructAsyncClientInstance` is kept (it still creates a manager that the caller cannot stop), but it now delegates to the new overload. An `IOException` from `new TAsyncClientManager()` now gives an `IllegalArgumentException` with the message `Thrift async client manager cannot be created` and the `IOException` as cause; before, the `IllegalArgumentException` only wrapped it. - An asynchronous call sent just before the producer stops now never gets its callback, so its exchange does not complete. In libthrift 0.25.0 `stop()` ends the selector thread (`SelectThread.finish()` sets `running` to false and wakes the selector), and `run()` then only closes the selector (javap): a call that has not started yet or is registered on the selector gets neither `onComplete` nor `onError`. On main a call that had not started yet failed with `ClosedChannelException` through `onError` (the transport was already closed), and a call already registered hung as well. A graceful shutdown waits for the inflight exchanges first, so this only happens when a producer is evicted from a cache or the shutdown timeout runs out. A timeout on the async client would not help, as the selector thread itself times the calls out. A call made after the stop fails with `TException: SelectThread is not running`, which is set on the exchange. - No option or generated file change. No upgrade guide entry: nothing for users to change. - Backport: camel-4.23.x, 4.22.x, 4.18.x and 4.14.x have the same code. Tests: - New `ThriftProducerAsyncStopTest`, JUnit assertions as in the module. It restarts a route with an asynchronous `ping` producer 5 times, sends a ping after each restart (the asynchronous producer of the restarted route works), stops the route and waits up to 5 s for the selector threads created during the test to end. - `routeRestartsStopTheClientManagersOfTheStoppedProducers` fails on main: `selector threads of stopped asynchronous producers ==> expected: <0> but was: <5>` (one per restart; the thread of the first producer existed before the test). - Unit tests of camel-thrift with the fix: 56 run, 0 failures, 0 skipped. - The formatter and import-sort plugins ran. I built offline with `-Dasm-version=9.11` because asm 9.11.1 was not in my local repository. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] 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): [CAMEL-26050](https://issues.apache.org/jira/browse/CAMEL-26050). # 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-thrift and ran its tests, including the formatter and import-sort plugins. No generated file changes. 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]
