You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Robert Munteanu (JIRA)" <ji...@apache.org> on 2016/01/08 15:20:39 UTC
[jira] [Commented] (SLING-5416) Thread Pool should stop
"gracefully"
[ https://issues.apache.org/jira/browse/SLING-5416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15089252#comment-15089252 ]
Robert Munteanu commented on SLING-5416:
----------------------------------------
I guess the danger here is that the shutdown blocks indefinitely. Maybe shipping a default configuration with
- shutdown graceful: true
- shutdown wait time: 5000
would be a better compromise?
> Thread Pool should stop "gracefully"
> ------------------------------------
>
> Key: SLING-5416
> URL: https://issues.apache.org/jira/browse/SLING-5416
> Project: Sling
> Issue Type: Improvement
> Components: Commons
> Reporter: Thomas Mueller
>
> By default, the Sling thread pool manager calls "Thread.interrupt" to close the thread pool. This is because it is configured to be "not graceful" by default:
> {noformat}
> ./bundles/commons/threads/src/main/java/org/apache/sling/commons/threads/ModifiableThreadPoolConfig.java
> ...
> * The default values for this configuration are:
> ...
> * - shutdown graceful: false
> * - shutdown wait time: -1
> ...
>
> ./bundles/commons/threads/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPool.java
> public void shutdown() {
> this.logger.info("Shutting down thread pool [{}] ...", name);
> if ( this.executor != null ) {
> if (this.configuration.isShutdownGraceful()) {
> this.executor.shutdown();
> } else {
> this.executor.shutdownNow();
> }
> try {
> if (this.configuration.getShutdownWaitTimeMs() > 0) {
> if (!this.executor.awaitTermination(this.configuration.getShutdownWaitTimeMs(), TimeUnit.MILLISECONDS)) {
> logger.warn("Running commands have not terminated within "
> + this.configuration.getShutdownWaitTimeMs()
> + "ms. Will shut them down by interruption");
> this.executor.shutdownNow(); // TODO: shouldn't this be outside the if statement?!
> }
> }
> } catch (final InterruptedException ie) {
> this.logger.error("Cannot shutdown thread pool [" + this.name + "]", ie);
> }
> {noformat}
> I think Sling should change the default to be graceful (not call Thread.interrupt()). Thread.interrupt can can result in many problems, because it closes channels, possibly TCP/IP connections, and so on. When using external libraries (JDBC, MongoDB, Apache Lucene,...) it is hard to ensure that Thread.interrupt does not cause problems.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)