allthingssecurity opened a new pull request, #29121: URL: https://github.com/apache/camel/pull/29121
## CAMEL-26537: camel-core - TimerListenerManager: adding or removing a listener while the timer runs must not stop the timer `TimerListenerManager` keeps its listeners in a `LinkedHashSet`. The timer thread iterates the set in `run()`, while `addTimerListener` and `removeTimerListener` change it with no lock, from other threads or from a listener running on the timer thread. If the set changes while the timer is in a listener that is not the last one, the iterator throws a `ConcurrentModificationException`. The `try` is around each listener call, not around the loop, so `run()` throws and `scheduleAtFixedRate` never runs it again. The timer stops for good, and nothing is logged. ``` 1 timer thread : run() -> iterator.next() = A, A.onTimer() is running (B is next) 2 A itself, or another thread : addTimerListener(C) (or removeTimerListener(B)) 3 timer thread : A.onTimer() returns -> iterator.next() -> ConcurrentModificationException ``` ### The trigger that needs no race This happens when two things combine: - A vault refresh task (AWS Secrets Manager, HashiCorp Vault, Spring Cloud Config, `camel.vault.*.refreshEnabled`) reloads the routes from its `run()`, on the timer thread. - A restarted `ServiceBusConsumer` schedules its lock renewer in `doStart` (default `PEEK_LOCK` and `maxAutoLockRenewDuration`). The refresh task is scheduled before any route starts, and the renewer from the consumer's first start is never removed. So the refresh task is never the last listener, and the first secret rotation stops the timer. After that: - no lock is renewed, so long exchanges are delivered again; - no further secret rotation is picked up. I traced this in the code and reproduced it with the tests below. I did not run it against a real vault and Service Bus. ### Other triggers - A period task scheduled from another thread, such as a Service Bus route started at runtime. - The JMX load statistics (`loadStatisticsEnabled`): their listeners are added and removed when routes are added, removed or reloaded. - Two threads adding at the same time can also lose listeners, because `LinkedHashSet` is not thread safe. ### Fix Keep the listeners in a `CopyOnWriteArraySet`, as `DefaultBackOffTimer` and `DefaultTaskManagerRegistry` do for their tasks. - It keeps the insertion order, which `DefaultPeriodTaskScheduler.getTaskByType` relies on. - The timer, the dev consoles and the CLI iterate a snapshot, so a change no longer breaks a run in progress. - Listeners change rarely compared with how often the timer ticks, so copying the set on each change is cheap. A run that has already started can still call a listener that was removed during that run, once. By then `removeTimerListener` has already stopped it. ### Tests - `TimerListenerManagerTest`: a latch holds the timer thread in the first listener while the test adds a listener (or removes the next one). The timer must keep calling the listeners, including the new one. - `PeriodTaskSchedulerTest`: the same with `schedulePeriodTask`, and a period task that schedules another task from its own run (the reload case above). All 4 tests fail on main and pass with the fix. Run 20 times each, none failed. The camel-core timer, period and engine tests and the camel-management tests show no new failures. ### Follow-ups (I will file them) - `PeriodTaskScheduler` cannot remove a task, so `ServiceBusConsumer` leaves its cancelled renewer behind on each restart. - `run()` catches only `Exception`. An `Error` thrown by a listener still stops the timer, with nothing logged. ### Backport The change cherry-picks cleanly onto 4.23.x, 4.22.x and 4.18.x, which have the same code and the Service Bus renewer. 4.14.x has the same code, but only the JMX statistics are affected there. _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]
