jbonofre commented on code in PR #2911:
URL: https://github.com/apache/karaf/pull/2911#discussion_r4191778292
##########
util/src/main/java/org/apache/karaf/util/tracker/BaseActivator.java:
##########
@@ -110,7 +117,9 @@ public void stop(BundleContext context) throws Exception {
scheduled.set(true);
doClose();
executor.shutdown();
- executor.awaitTermination(schedulerStopTimeout, TimeUnit.MILLISECONDS);
+ if (!executor.awaitTermination(schedulerStopTimeout,
TimeUnit.MILLISECONDS)) {
+ logger.warn("Executor did not terminate within {} milliseconds",
schedulerStopTimeout);
+ }
Review Comment:
This warning also first when the timeout is 0, which is a supported setup.
The `feature/core` `Activator` calls `setSchedulerStopTimeout(0)` on purpose,
so that `stop()` does not wait for the current job (otherwise the features
service could not refresh itself). In that case `awaitTermination(0)` returns
`false` as soon as a job is still running, and we log "Executor did not
terminate within 0 milliseconds" on a flow that was silent before.
Could you only log when a wait was actually requested?
```suggestion
if (!executor.awaitTermination(schedulerStopTimeout,
TimeUnit.MILLISECONDS) && schedulerStopTimeout > 0) {
logger.warn("Executor did not terminate within {} milliseconds",
schedulerStopTimeout);
}
```
The suggestion keeps the `awaitTermination` call first, so the behavior for
a zero timeout is exactly what it was before the PR.
--
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]
