gnodet commented on code in PR #22343:
URL: https://github.com/apache/camel/pull/22343#discussion_r3010742165
##########
components/camel-jcr/src/main/java/org/apache/camel/component/jcr/JcrConsumer.java:
##########
@@ -157,20 +156,23 @@ private void unregisterListenerAndLogoutSession() throws
RepositoryException {
}
private void cancelSessionListenerChecker() {
- if (sessionListenerCheckerScheduledFuture != null) {
- sessionListenerCheckerScheduledFuture.cancel(true);
+ if (sessionListenerCheckerExecutor != null) {
+ getJcrEndpoint().getCamelContext().getExecutorServiceManager()
+ .shutdownNow(sessionListenerCheckerExecutor);
+ sessionListenerCheckerExecutor = null;
}
}
private void scheduleSessionListenerChecker() {
String name = "JcrConsumerSessionChecker[" +
getJcrEndpoint().getEndpointConfiguredDestinationName() + "]";
- ScheduledExecutorService executor =
getJcrEndpoint().getCamelContext().getExecutorServiceManager()
+ sessionListenerCheckerExecutor =
getJcrEndpoint().getCamelContext().getExecutorServiceManager()
.newSingleThreadScheduledExecutor(this, name);
JcrConsumerSessionListenerChecker sessionListenerChecker = new
JcrConsumerSessionListenerChecker();
long sessionLiveCheckIntervalOnStart =
JcrConsumer.this.getJcrEndpoint().getSessionLiveCheckIntervalOnStart();
long sessionLiveCheckInterval =
JcrConsumer.this.getJcrEndpoint().getSessionLiveCheckInterval();
- sessionListenerCheckerScheduledFuture =
executor.scheduleWithFixedDelay(sessionListenerChecker,
- sessionLiveCheckIntervalOnStart, sessionLiveCheckInterval,
TimeUnit.MILLISECONDS);
+ sessionListenerCheckerExecutor.scheduleWithFixedDelay(
+ sessionListenerChecker, sessionLiveCheckIntervalOnStart,
sessionLiveCheckInterval,
+ TimeUnit.MILLISECONDS);
Review Comment:
The `sessionListenerCheckerExecutor` is managed by two methods:
`scheduleSessionListenerChecker` (called in `doStart`) and
`cancelSessionListenerChecker` (called in `doStop`).
The original problem was that the `ScheduledExecutorService` was created as
a **local variable** in `scheduleSessionListenerChecker()` — only the
`ScheduledFuture` was stored and cancelled in `doStop`. The executor itself was
never shut down, which is the resource leak SonarCloud flagged.
The fix stores the executor in a field instead and shuts it down properly
via `ExecutorServiceManager.shutdownNow()` in `cancelSessionListenerChecker()`.
Since `shutdownNow()` cancels all scheduled tasks, the separate
`ScheduledFuture` field is no longer needed.
Regarding your question about the guard: since
`scheduleSessionListenerChecker()` is only called from `doStart()` and
`cancelSessionListenerChecker()` from `doStop()`, Camel's lifecycle guarantees
they are called in order, so the executor should always be null when entering
`scheduleSessionListenerChecker()`. But I can add a defensive check if you
prefer.
_Claude Code on behalf of Guillaume Nodet_
--
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]
