allthingssecurity opened a new pull request, #28535: URL: https://github.com/apache/camel/pull/28535
# Description [CAMEL-26048](https://issues.apache.org/jira/browse/CAMEL-26048) `MinaConsumer` builds its acceptor (or, in client mode, its connector) and its worker pool in the constructor. `doStop` only unbinds the address (client mode: disposes the connector); `acceptor.dispose(true)` and `workerPool.shutdown()` are only in `doShutdown`. A route that is stopped and started again does not shut its consumer down: `RouteService` stops it, and the next start creates a new consumer. So the old consumer's `doShutdown` never runs, and every route restart leaks the stopped acceptor: - tcp: a `NioSocketAcceptor` with its acceptor thread (not a daemon thread) and one selector per I/O processor (`availableProcessors() + 1`), plus the worker pool; - udp: a `NioDatagramAcceptor` and its worker pool (non-daemon pool threads); - vm: a `VmPipeAcceptor` and its idle status checker thread (not a daemon thread). Measured on a 10-core machine with 5 route restarts of a tcp consumer: 5 more `NioSocketAcceptor-N` threads and about 210 more open file descriptors, never released. The non-daemon threads also keep the JVM running after the CamelContext is stopped, as only the current consumer is shut down then. Triggers: stopping and starting a route (JMX, control bus, dev console, `RouteController`), route policies that stop and start their route, and the start retries of the `SupervisingRouteController` (each failed attempt creates a new consumer, which is never shut down). CAMEL-9738 added the dispose to `doShutdown`, which this path never reaches. Removing a route (route reload, route templates and kamelets created again) does shut the current consumer down, so there only the consumers left over from earlier restarts leak. Changes: - `MinaConsumer.doStop` disposes the acceptor after the unbind, and shuts the worker pool down (also in client mode, where only the connector was disposed). - `doStop` waits for the I/O threads of the acceptor to end (`dispose(true)`) only when the CamelContext is stopping, as `doShutdown` did before, so `CamelContext.stop()` still waits for them (a datagram acceptor takes about two seconds, as on main). A route stop does not wait (`dispose(false)`): the address is already unbound, and the threads end on their own. Both mark the acceptor disposed (javap `AbstractIoService.dispose`, mina-core 2.2.9), so the `dispose(true)` in `doShutdown`, which is unchanged, then does nothing. - The protocol setup moved from the constructor to a method that `doStart` runs again when the consumer was stopped. So a consumer that is stopped and started again (not replaced) gets a new acceptor or connector. This is the path of route and CamelContext suspend/resume (`MinaConsumer` is not `Suspendable`, so it is stopped and started) and of route policies that stop the consumer itself. On main such a restart bound the old acceptor again, and in client mode it failed with `IllegalStateException: The connector is being disposed.` - A package-private `getConnector()` for the test of client mode. - 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 consumer. Tests: - New `MinaConsumerRestartTest`, 6 tests, JUnit assertions as in the module (Mockito for one, as in `MinaProducerShutdownMockTest`). All 6 fail on main: 5 with `expected: <true> but was: <false>` (for example `the acceptor of a stopped consumer must be disposed`) or, for the Mockito test, `Wanted but not invoked: ioAcceptor.dispose(false)`, and the client mode one with `IllegalStateException: The connector is being disposed.`: - `routeRestartReleasesTheAcceptorOfTheStoppedTcpConsumer` and `routeRestartReleasesTheAcceptorOfTheStoppedUdpConsumer`: 3 route restarts; each stopped acceptor is disposed and its worker pool is shut down, and the restarted route receives a message. - `routeRestartReleasesTheAcceptorOfTheStoppedVmConsumer`: each stopped acceptor is disposed (vm has no worker pool), and the restarted route receives a message. - `theSameConsumerCanBeStartedAgain`: `consumer.stop()` then `consumer.start()` on the same instance gives a new, working acceptor. - `theSameClientModeConsumerCanBeStartedAgain`: the same for a consumer in client mode connected to the tcp route: the old connector is disposed and its worker pool shut down, and the new connector has a session. - `onlyACamelContextStopWaitsForTheIoThreads`: the udp acceptor behind a Mockito mock that passes every call on to it; a route stop calls `dispose(false)`, a CamelContext stop `dispose(true)`. With `dispose(false)` in both cases only this test fails (negative control of the wait). - The udp route is only added in the two udp tests, as each CamelContext stop then waits about two seconds for the datagram acceptor. - Unit tests of camel-mina with the fix: 122 run, 0 failures, 4 skipped. A throw-away check (not in the PR) also saw no `NioSocketAcceptor` or `NioProcessor` thread left right after 5 restarts and a stop of a tcp route. - 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. Not changed: the producer with `sync=true` (pooled) creates a worker pool per producer that only `doShutdown` shuts down; an evicted producer's pool threads end after their 30 s keep-alive. I can open a JIRA for it. # 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-26048](https://issues.apache.org/jira/browse/CAMEL-26048). # 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-mina 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]
