[tomcat] 01/02: Improve locking of utility executor
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit ea9a80fe743fca86362a2184806941b655bd0088
Author: Mark Thomas
AuthorDate: Thu May 4 11:41:57 2023 +0100
Improve locking of utility executor
Fixes some edge cases around calling setUtilityThreads() during stop
---
java/org/apache/catalina/core/StandardServer.java | 51 +--
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/java/org/apache/catalina/core/StandardServer.java
b/java/org/apache/catalina/core/StandardServer.java
index 8db5d8f05a..f7ac2d2e61 100644
--- a/java/org/apache/catalina/core/StandardServer.java
+++ b/java/org/apache/catalina/core/StandardServer.java
@@ -429,27 +429,30 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
return;
}
this.utilityThreads = utilityThreads;
-if (oldUtilityThreads != utilityThreads && utilityExecutor != null) {
-
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+synchronized (utilityExecutorLock) {
+if (oldUtilityThreads != utilityThreads && utilityExecutor !=
null) {
+
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+}
}
}
+/*
+ * Callers must be holding utilityExecutorLock
+ */
private void reconfigureUtilityExecutor(int threads) {
-synchronized (utilityExecutorLock) {
-// The ScheduledThreadPoolExecutor doesn't use MaximumPoolSize,
only CorePoolSize is available
-if (utilityExecutor != null) {
-utilityExecutor.setCorePoolSize(threads);
-} else {
-ScheduledThreadPoolExecutor scheduledThreadPoolExecutor = new
ScheduledThreadPoolExecutor(threads,
-new TaskThreadFactory("Catalina-utility-",
utilityThreadsAsDaemon, Thread.MIN_PRIORITY));
-scheduledThreadPoolExecutor.setKeepAliveTime(10,
TimeUnit.SECONDS);
-scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
-
scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
-utilityExecutor = scheduledThreadPoolExecutor;
-utilityExecutorWrapper = new
org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor(
-utilityExecutor);
-}
+// The ScheduledThreadPoolExecutor doesn't use MaximumPoolSize, only
CorePoolSize is available
+if (utilityExecutor != null) {
+utilityExecutor.setCorePoolSize(threads);
+} else {
+ScheduledThreadPoolExecutor scheduledThreadPoolExecutor = new
ScheduledThreadPoolExecutor(threads,
+new TaskThreadFactory("Catalina-utility-",
utilityThreadsAsDaemon, Thread.MIN_PRIORITY));
+scheduledThreadPoolExecutor.setKeepAliveTime(10, TimeUnit.SECONDS);
+scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
+
scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
+utilityExecutor = scheduledThreadPoolExecutor;
+utilityExecutorWrapper = new
org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor(
+utilityExecutor);
}
}
@@ -981,8 +984,10 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
super.initInternal();
// Initialize utility executor
-reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
-register(utilityExecutor, "type=UtilityExecutor");
+synchronized (utilityExecutorLock) {
+
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+register(utilityExecutor, "type=UtilityExecutor");
+}
// Register global String cache
// Note although the cache is global, if there are multiple Servers
@@ -1042,10 +1047,12 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
unregister(onameStringCache);
-if (utilityExecutor != null) {
-utilityExecutor.shutdownNow();
-unregister("type=UtilityExecutor");
-utilityExecutor = null;
+synchronized (utilityExecutorLock) {
+if (utilityExecutor != null) {
+utilityExecutor.shutdownNow();
+unregister("type=UtilityExecutor");
+utilityExecutor = null;
+}
}
super.destroyInternal();
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit de6aacea433f28786329ac14f32d41a4d9951ebb
Author: Mark Thomas
AuthorDate: Thu May 4 14:41:01 2023 +0100
Move management of utility executor from init/destroy to start/stop
---
java/org/apache/catalina/connector/Connector.java | 14 ---
java/org/apache/catalina/core/ContainerBase.java | 20 +++-
java/org/apache/catalina/core/StandardServer.java | 28 +++---
.../apache/catalina/ha/tcp/SimpleTcpCluster.java | 5 +++-
webapps/docs/changelog.xml | 5
5 files changed, 42 insertions(+), 30 deletions(-)
diff --git a/java/org/apache/catalina/connector/Connector.java
b/java/org/apache/catalina/connector/Connector.java
index c504210b64..7339f961b2 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -982,9 +982,6 @@ public class Connector extends LifecycleMBeanBase {
// Initialize adapter
adapter = new CoyoteAdapter(this);
protocolHandler.setAdapter(adapter);
-if (service != null) {
-
protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
-}
// Make sure parseBodyMethodsSet has a default
if (null == parseBodyMethodsSet) {
@@ -1033,9 +1030,15 @@ public class Connector extends LifecycleMBeanBase {
setState(LifecycleState.STARTING);
+// Configure the utility executor before starting the protocol handler
+if (protocolHandler != null && service != null) {
+
protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
+}
+
try {
protocolHandler.start();
} catch (Exception e) {
+// Includes NPE - protocolHandler will be null for invalid
protocol if throwOnFailure is false
throw new
LifecycleException(sm.getString("coyoteConnector.protocolHandlerStartFailed"),
e);
}
}
@@ -1058,6 +1061,11 @@ public class Connector extends LifecycleMBeanBase {
} catch (Exception e) {
throw new
LifecycleException(sm.getString("coyoteConnector.protocolHandlerStopFailed"),
e);
}
+
+ // Remove the utility executor once the protocol handler has been
stopped
+if (protocolHandler != null) {
+protocolHandler.setUtilityExecutor(null);
+}
}
diff --git a/java/org/apache/catalina/core/ContainerBase.java
b/java/org/apache/catalina/core/ContainerBase.java
index 7c96f5de5e..c5bc5994a6 100644
--- a/java/org/apache/catalina/core/ContainerBase.java
+++ b/java/org/apache/catalina/core/ContainerBase.java
@@ -820,13 +820,6 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
}
-@Override
-protected void initInternal() throws LifecycleException {
-reconfigureStartStopExecutor(getStartStopThreads());
-super.initInternal();
-}
-
-
private void reconfigureStartStopExecutor(int threads) {
if (threads == 1) {
// Use a fake executor
@@ -852,6 +845,8 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
@Override
protected synchronized void startInternal() throws LifecycleException {
+reconfigureStartStopExecutor(getStartStopThreads());
+
// Start our subordinate components, if any
logger = null;
getLogger();
@@ -958,6 +953,12 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
if (cluster instanceof Lifecycle) {
((Lifecycle) cluster).stop();
}
+
+// If init fails, this may be null
+if (startStopExecutor != null) {
+startStopExecutor.shutdownNow();
+startStopExecutor = null;
+}
}
@Override
@@ -987,11 +988,6 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
parent.removeChild(this);
}
-// If init fails, this may be null
-if (startStopExecutor != null) {
-startStopExecutor.shutdownNow();
-}
-
super.destroyInternal();
}
diff --git a/java/org/apache/catalina/core/StandardServer.java
b/java/org/apache/catalina/core/StandardServer.java
index f7ac2d2e61..00e70c8b48 100644
--- a/java/org/apache/catalina/core/StandardServer.java
+++ b/java/org/apache/catalina/core/StandardServer.java
@@ -909,6 +909,12 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
fireLifecycleEvent(CONFIGURE_START_EVENT, null);
setState(LifecycleState.STARTING);
+ // Initialize utility executor
+synchronized (utilityExecutorLock) {
+
reconfigureUtilityExecut
[tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 9c3e2c8148a513bcd2813761632988d82126df9d
Author: Mark Thomas
AuthorDate: Thu May 4 14:41:01 2023 +0100
Move management of utility executor from init/destroy to start/stop
---
java/org/apache/catalina/connector/Connector.java | 14 ---
java/org/apache/catalina/core/ContainerBase.java | 20 +++-
java/org/apache/catalina/core/StandardServer.java | 28 +++---
.../apache/catalina/ha/tcp/SimpleTcpCluster.java | 5 +++-
webapps/docs/changelog.xml | 9 +++
5 files changed, 46 insertions(+), 30 deletions(-)
diff --git a/java/org/apache/catalina/connector/Connector.java
b/java/org/apache/catalina/connector/Connector.java
index f9893b992e..18e865e157 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -994,9 +994,6 @@ public class Connector extends LifecycleMBeanBase {
// Initialize adapter
adapter = new CoyoteAdapter(this);
protocolHandler.setAdapter(adapter);
-if (service != null) {
-
protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
-}
// Make sure parseBodyMethodsSet has a default
if (null == parseBodyMethodsSet) {
@@ -1037,9 +1034,15 @@ public class Connector extends LifecycleMBeanBase {
setState(LifecycleState.STARTING);
+// Configure the utility executor before starting the protocol handler
+if (protocolHandler != null && service != null) {
+
protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
+}
+
try {
protocolHandler.start();
} catch (Exception e) {
+// Includes NPE - protocolHandler will be null for invalid
protocol if throwOnFailure is false
throw new
LifecycleException(sm.getString("coyoteConnector.protocolHandlerStartFailed"),
e);
}
}
@@ -1062,6 +1065,11 @@ public class Connector extends LifecycleMBeanBase {
} catch (Exception e) {
throw new
LifecycleException(sm.getString("coyoteConnector.protocolHandlerStopFailed"),
e);
}
+
+ // Remove the utility executor once the protocol handler has been
stopped
+if (protocolHandler != null) {
+protocolHandler.setUtilityExecutor(null);
+}
}
diff --git a/java/org/apache/catalina/core/ContainerBase.java
b/java/org/apache/catalina/core/ContainerBase.java
index 7c96f5de5e..c5bc5994a6 100644
--- a/java/org/apache/catalina/core/ContainerBase.java
+++ b/java/org/apache/catalina/core/ContainerBase.java
@@ -820,13 +820,6 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
}
-@Override
-protected void initInternal() throws LifecycleException {
-reconfigureStartStopExecutor(getStartStopThreads());
-super.initInternal();
-}
-
-
private void reconfigureStartStopExecutor(int threads) {
if (threads == 1) {
// Use a fake executor
@@ -852,6 +845,8 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
@Override
protected synchronized void startInternal() throws LifecycleException {
+reconfigureStartStopExecutor(getStartStopThreads());
+
// Start our subordinate components, if any
logger = null;
getLogger();
@@ -958,6 +953,12 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
if (cluster instanceof Lifecycle) {
((Lifecycle) cluster).stop();
}
+
+// If init fails, this may be null
+if (startStopExecutor != null) {
+startStopExecutor.shutdownNow();
+startStopExecutor = null;
+}
}
@Override
@@ -987,11 +988,6 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
parent.removeChild(this);
}
-// If init fails, this may be null
-if (startStopExecutor != null) {
-startStopExecutor.shutdownNow();
-}
-
super.destroyInternal();
}
diff --git a/java/org/apache/catalina/core/StandardServer.java
b/java/org/apache/catalina/core/StandardServer.java
index 227784dbb3..5459131810 100644
--- a/java/org/apache/catalina/core/StandardServer.java
+++ b/java/org/apache/catalina/core/StandardServer.java
@@ -905,6 +905,12 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
fireLifecycleEvent(CONFIGURE_START_EVENT, null);
setState(LifecycleState.STARTING);
+ // Initialize utility executor
+synchronized (utilityExecutorLock) {
+
reconfigureUtilityExecut
[tomcat] 01/02: Improve locking of utility executor
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 17b76c618145b6d431ade2d1844a65bf04f9ece7
Author: Mark Thomas
AuthorDate: Thu May 4 11:41:57 2023 +0100
Improve locking of utility executor
Fixes some edge cases around calling setUtilityThreads() during stop
---
java/org/apache/catalina/core/StandardServer.java | 51 +--
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/java/org/apache/catalina/core/StandardServer.java
b/java/org/apache/catalina/core/StandardServer.java
index 5f4637153b..227784dbb3 100644
--- a/java/org/apache/catalina/core/StandardServer.java
+++ b/java/org/apache/catalina/core/StandardServer.java
@@ -425,27 +425,30 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
return;
}
this.utilityThreads = utilityThreads;
-if (oldUtilityThreads != utilityThreads && utilityExecutor != null) {
-
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+synchronized (utilityExecutorLock) {
+if (oldUtilityThreads != utilityThreads && utilityExecutor !=
null) {
+
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+}
}
}
+/*
+ * Callers must be holding utilityExecutorLock
+ */
private void reconfigureUtilityExecutor(int threads) {
-synchronized (utilityExecutorLock) {
-// The ScheduledThreadPoolExecutor doesn't use MaximumPoolSize,
only CorePoolSize is available
-if (utilityExecutor != null) {
-utilityExecutor.setCorePoolSize(threads);
-} else {
-ScheduledThreadPoolExecutor scheduledThreadPoolExecutor = new
ScheduledThreadPoolExecutor(threads,
-new TaskThreadFactory("Catalina-utility-",
utilityThreadsAsDaemon, Thread.MIN_PRIORITY));
-scheduledThreadPoolExecutor.setKeepAliveTime(10,
TimeUnit.SECONDS);
-scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
-
scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
-utilityExecutor = scheduledThreadPoolExecutor;
-utilityExecutorWrapper = new
org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor(
-utilityExecutor);
-}
+// The ScheduledThreadPoolExecutor doesn't use MaximumPoolSize, only
CorePoolSize is available
+if (utilityExecutor != null) {
+utilityExecutor.setCorePoolSize(threads);
+} else {
+ScheduledThreadPoolExecutor scheduledThreadPoolExecutor = new
ScheduledThreadPoolExecutor(threads,
+new TaskThreadFactory("Catalina-utility-",
utilityThreadsAsDaemon, Thread.MIN_PRIORITY));
+scheduledThreadPoolExecutor.setKeepAliveTime(10, TimeUnit.SECONDS);
+scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
+
scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
+utilityExecutor = scheduledThreadPoolExecutor;
+utilityExecutorWrapper = new
org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor(
+utilityExecutor);
}
}
@@ -977,8 +980,10 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
super.initInternal();
// Initialize utility executor
-reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
-register(utilityExecutor, "type=UtilityExecutor");
+synchronized (utilityExecutorLock) {
+
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+register(utilityExecutor, "type=UtilityExecutor");
+}
// Register global String cache
// Note although the cache is global, if there are multiple Servers
@@ -1013,10 +1018,12 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
unregister(onameStringCache);
-if (utilityExecutor != null) {
-utilityExecutor.shutdownNow();
-unregister("type=UtilityExecutor");
-utilityExecutor = null;
+synchronized (utilityExecutorLock) {
+if (utilityExecutor != null) {
+utilityExecutor.shutdownNow();
+unregister("type=UtilityExecutor");
+utilityExecutor = null;
+}
}
super.destroyInternal();
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
On 05/05/2023 13:17, Han Li wrote:
On May 5, 2023, at 18:42, Mark Thomas wrote:
On 05/05/2023 04:21, Han Li wrote:
On May 4, 2023, at 21:41, ma...@apache.org wrote:
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4b097bf2e9075e9e2949ec5aa410cba3c2b85374
Author: Mark Thomas
AuthorDate: Thu May 4 14:41:01 2023 +0100
Move management of utility executor from init/destroy to start/stop
---
java/org/apache/catalina/connector/Connector.java | 13 +++---
java/org/apache/catalina/core/ContainerBase.java | 20 +++-
java/org/apache/catalina/core/StandardServer.java | 28 +++---
.../apache/catalina/ha/tcp/SimpleTcpCluster.java | 5 +++-
webapps/docs/changelog.xml | 5
5 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/java/org/apache/catalina/connector/Connector.java
b/java/org/apache/catalina/connector/Connector.java
index c9200e20ca..dac7fdd642 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -992,9 +992,6 @@ public class Connector extends LifecycleMBeanBase {
// Initialize adapter
adapter = new CoyoteAdapter(this);
protocolHandler.setAdapter(adapter);
- if (service != null) {
- protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
- }
// Make sure parseBodyMethodsSet has a default
if (null == parseBodyMethodsSet) {
@@ -1035,6 +1032,11 @@ public class Connector extends LifecycleMBeanBase {
setState(LifecycleState.STARTING);
+ // Configure the utility executor before starting the protocol handler
+ if (service != null) {
+ protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
According to check logic at line 1027, the protocalHandler may be null, so need
NPE check.
I'm not convinced that check is necessary given the call to
protocalHandler.start() just below. I need to look into this more to see why
the null check is there.
I have also looked into this and found which related to
org.apache.catalina.connector.TestConnector#doTestInvalidProtocol.
The reason that why this has three conditions:
1. The protocol is invalid
2. The thorwOnFailure has been set false
2) lead the null check in initInternal method has invalid and go on to
startInternal.
Thanks. That is helpful. I'll add some null checks for the utility
executor calls.
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
> On May 5, 2023, at 18:42, Mark Thomas wrote:
>
> On 05/05/2023 04:21, Han Li wrote:
>>> On May 4, 2023, at 21:41, ma...@apache.org wrote:
>>>
>>> This is an automated email from the ASF dual-hosted git repository.
>>>
>>> markt pushed a commit to branch main
>>> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>>>
>>> commit 4b097bf2e9075e9e2949ec5aa410cba3c2b85374
>>> Author: Mark Thomas
>>> AuthorDate: Thu May 4 14:41:01 2023 +0100
>>>
>>> Move management of utility executor from init/destroy to start/stop
>>> ---
>>> java/org/apache/catalina/connector/Connector.java | 13 +++---
>>> java/org/apache/catalina/core/ContainerBase.java | 20 +++-
>>> java/org/apache/catalina/core/StandardServer.java | 28
>>> +++---
>>> .../apache/catalina/ha/tcp/SimpleTcpCluster.java | 5 +++-
>>> webapps/docs/changelog.xml | 5
>>> 5 files changed, 41 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/java/org/apache/catalina/connector/Connector.java
>>> b/java/org/apache/catalina/connector/Connector.java
>>> index c9200e20ca..dac7fdd642 100644
>>> --- a/java/org/apache/catalina/connector/Connector.java
>>> +++ b/java/org/apache/catalina/connector/Connector.java
>>> @@ -992,9 +992,6 @@ public class Connector extends LifecycleMBeanBase {
>>> // Initialize adapter
>>> adapter = new CoyoteAdapter(this);
>>> protocolHandler.setAdapter(adapter);
>>> - if (service != null) {
>>> -
>>> protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
>>> - }
>>>
>>> // Make sure parseBodyMethodsSet has a default
>>> if (null == parseBodyMethodsSet) {
>>> @@ -1035,6 +1032,11 @@ public class Connector extends LifecycleMBeanBase {
>>>
>>> setState(LifecycleState.STARTING);
>>>
>>> + // Configure the utility executor before starting the protocol handler
>>> + if (service != null) {
>>> +
>>> protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
>> According to check logic at line 1027, the protocalHandler may be null, so
>> need NPE check.
>
> I'm not convinced that check is necessary given the call to
> protocalHandler.start() just below. I need to look into this more to see why
> the null check is there.
I have also looked into this and found which related to
org.apache.catalina.connector.TestConnector#doTestInvalidProtocol.
The reason that why this has three conditions:
1. The protocol is invalid
2. The thorwOnFailure has been set false
2) lead the null check in initInternal method has invalid and go on to
startInternal.
Han
>
>>> + }
>>> +
>>> try {
>>> protocolHandler.start();
>>> } catch (Exception e) {
>>> @@ -1060,6 +1062,11 @@ public class Connector extends LifecycleMBeanBase {
>>> } catch (Exception e) {
>>> throw new
>>> LifecycleException(sm.getString("coyoteConnector.protocolHandlerStopFailed"),
>>> e);
>>> }
>>> +
>>> + // Remove the utility executor once the protocol handler has been stopped
>>> + if (service != null) {
>>> + protocolHandler.setUtilityExecutor(null);
>> Same as above.
>
> I agree on this one.
>
> Mark
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> <mailto:dev-unsubscr...@tomcat.apache.org>
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> <mailto:dev-h...@tomcat.apache.org>
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
Hi Team ,
Management of start and stop might be arrested
Regards
Koti
On Thu, 4 May 2023, 19:12 , wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch main
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
> commit 4b097bf2e9075e9e2949ec5aa410cba3c2b85374
> Author: Mark Thomas
> AuthorDate: Thu May 4 14:41:01 2023 +0100
>
> Move management of utility executor from init/destroy to start/stop
> ---
> java/org/apache/catalina/connector/Connector.java | 13 +++---
> java/org/apache/catalina/core/ContainerBase.java | 20 +++-
> java/org/apache/catalina/core/StandardServer.java | 28
> +++---
> .../apache/catalina/ha/tcp/SimpleTcpCluster.java | 5 +++-
> webapps/docs/changelog.xml | 5
> 5 files changed, 41 insertions(+), 30 deletions(-)
>
> diff --git a/java/org/apache/catalina/connector/Connector.java
> b/java/org/apache/catalina/connector/Connector.java
> index c9200e20ca..dac7fdd642 100644
> --- a/java/org/apache/catalina/connector/Connector.java
> +++ b/java/org/apache/catalina/connector/Connector.java
> @@ -992,9 +992,6 @@ public class Connector extends LifecycleMBeanBase {
> // Initialize adapter
> adapter = new CoyoteAdapter(this);
> protocolHandler.setAdapter(adapter);
> -if (service != null) {
> -
> protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
> -}
>
> // Make sure parseBodyMethodsSet has a default
> if (null == parseBodyMethodsSet) {
> @@ -1035,6 +1032,11 @@ public class Connector extends LifecycleMBeanBase {
>
> setState(LifecycleState.STARTING);
>
> +// Configure the utility executor before starting the protocol
> handler
> +if (service != null) {
> +
> protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
> +}
> +
> try {
> protocolHandler.start();
> } catch (Exception e) {
> @@ -1060,6 +1062,11 @@ public class Connector extends LifecycleMBeanBase {
> } catch (Exception e) {
> throw new
> LifecycleException(sm.getString("coyoteConnector.protocolHandlerStopFailed"),
> e);
> }
> +
> +// Remove the utility executor once the protocol handler has been
> stopped
> +if (service != null) {
> +protocolHandler.setUtilityExecutor(null);
> +}
> }
>
>
> diff --git a/java/org/apache/catalina/core/ContainerBase.java
> b/java/org/apache/catalina/core/ContainerBase.java
> index 784c9032ef..a7e7c69a4a 100644
> --- a/java/org/apache/catalina/core/ContainerBase.java
> +++ b/java/org/apache/catalina/core/ContainerBase.java
> @@ -787,13 +787,6 @@ public abstract class ContainerBase extends
> LifecycleMBeanBase implements Contai
> }
>
>
> -@Override
> -protected void initInternal() throws LifecycleException {
> -reconfigureStartStopExecutor(getStartStopThreads());
> -super.initInternal();
> -}
> -
> -
> private void reconfigureStartStopExecutor(int threads) {
> if (threads == 1) {
> // Use a fake executor
> @@ -819,6 +812,8 @@ public abstract class ContainerBase extends
> LifecycleMBeanBase implements Contai
> @Override
> protected synchronized void startInternal() throws LifecycleException
> {
>
> +reconfigureStartStopExecutor(getStartStopThreads());
> +
> // Start our subordinate components, if any
> logger = null;
> getLogger();
> @@ -925,6 +920,12 @@ public abstract class ContainerBase extends
> LifecycleMBeanBase implements Contai
> if (cluster instanceof Lifecycle) {
> ((Lifecycle) cluster).stop();
> }
> +
> +// If init fails, this may be null
> +if (startStopExecutor != null) {
> +startStopExecutor.shutdownNow();
> +startStopExecutor = null;
> +}
> }
>
> @Override
> @@ -954,11 +955,6 @@ public abstract class ContainerBase extends
> LifecycleMBeanBase implements Contai
> parent.removeChild(this);
> }
>
> -// If init fails, this may be null
> -if (startStopExecutor != null) {
> -startStopExecutor.shutdownNow();
> -}
> -
> super.destroyInternal();
> }
>
> diff --git a/java/org/apache/catalina/core/StandardServer.java
> b/java/org/apache/catalina/core/StandardServer.java
> index 80b5026fed..a4383f2503 100644
> --- a/java/org/apache/catalina/core/StandardServer.java
> +
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
On 05/05/2023 04:21, Han Li wrote:
On May 4, 2023, at 21:41, ma...@apache.org wrote:
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4b097bf2e9075e9e2949ec5aa410cba3c2b85374
Author: Mark Thomas
AuthorDate: Thu May 4 14:41:01 2023 +0100
Move management of utility executor from init/destroy to start/stop
---
java/org/apache/catalina/connector/Connector.java | 13 +++---
java/org/apache/catalina/core/ContainerBase.java | 20 +++-
java/org/apache/catalina/core/StandardServer.java | 28 +++---
.../apache/catalina/ha/tcp/SimpleTcpCluster.java | 5 +++-
webapps/docs/changelog.xml | 5
5 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/java/org/apache/catalina/connector/Connector.java
b/java/org/apache/catalina/connector/Connector.java
index c9200e20ca..dac7fdd642 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -992,9 +992,6 @@ public class Connector extends LifecycleMBeanBase {
// Initialize adapter
adapter = new CoyoteAdapter(this);
protocolHandler.setAdapter(adapter);
-if (service != null) {
-
protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
-}
// Make sure parseBodyMethodsSet has a default
if (null == parseBodyMethodsSet) {
@@ -1035,6 +1032,11 @@ public class Connector extends LifecycleMBeanBase {
setState(LifecycleState.STARTING);
+// Configure the utility executor before starting the protocol handler
+if (service != null) {
+
protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
According to check logic at line 1027, the protocalHandler may be null, so
need NPE check.
I'm not convinced that check is necessary given the call to
protocalHandler.start() just below. I need to look into this more to see
why the null check is there.
+}
+
try {
protocolHandler.start();
} catch (Exception e) {
@@ -1060,6 +1062,11 @@ public class Connector extends LifecycleMBeanBase {
} catch (Exception e) {
throw new
LifecycleException(sm.getString("coyoteConnector.protocolHandlerStopFailed"),
e);
}
+
+// Remove the utility executor once the protocol handler has been
stopped
+if (service != null) {
+protocolHandler.setUtilityExecutor(null);
Same as above.
I agree on this one.
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
> On May 4, 2023, at 21:41, ma...@apache.org wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch main
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
> commit 4b097bf2e9075e9e2949ec5aa410cba3c2b85374
> Author: Mark Thomas
> AuthorDate: Thu May 4 14:41:01 2023 +0100
>
> Move management of utility executor from init/destroy to start/stop
> ---
> java/org/apache/catalina/connector/Connector.java | 13 +++---
> java/org/apache/catalina/core/ContainerBase.java | 20 +++-
> java/org/apache/catalina/core/StandardServer.java | 28 +++---
> .../apache/catalina/ha/tcp/SimpleTcpCluster.java | 5 +++-
> webapps/docs/changelog.xml | 5
> 5 files changed, 41 insertions(+), 30 deletions(-)
>
> diff --git a/java/org/apache/catalina/connector/Connector.java
> b/java/org/apache/catalina/connector/Connector.java
> index c9200e20ca..dac7fdd642 100644
> --- a/java/org/apache/catalina/connector/Connector.java
> +++ b/java/org/apache/catalina/connector/Connector.java
> @@ -992,9 +992,6 @@ public class Connector extends LifecycleMBeanBase {
> // Initialize adapter
> adapter = new CoyoteAdapter(this);
> protocolHandler.setAdapter(adapter);
> -if (service != null) {
> -
> protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
> -}
>
> // Make sure parseBodyMethodsSet has a default
> if (null == parseBodyMethodsSet) {
> @@ -1035,6 +1032,11 @@ public class Connector extends LifecycleMBeanBase {
>
> setState(LifecycleState.STARTING);
>
> +// Configure the utility executor before starting the protocol
> handler
> +if (service != null) {
> +
> protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
According to check logic at line 1027, the protocalHandler may be null, so
need NPE check.
> +}
> +
> try {
> protocolHandler.start();
> } catch (Exception e) {
> @@ -1060,6 +1062,11 @@ public class Connector extends LifecycleMBeanBase {
> } catch (Exception e) {
> throw new
> LifecycleException(sm.getString("coyoteConnector.protocolHandlerStopFailed"),
> e);
> }
> +
> +// Remove the utility executor once the protocol handler has been
> stopped
> +if (service != null) {
> +protocolHandler.setUtilityExecutor(null);
Same as above.
Han
> +}
> }
>
>
> diff --git a/java/org/apache/catalina/core/ContainerBase.java
> b/java/org/apache/catalina/core/ContainerBase.java
> index 784c9032ef..a7e7c69a4a 100644
> --- a/java/org/apache/catalina/core/ContainerBase.java
> +++ b/java/org/apache/catalina/core/ContainerBase.java
> @@ -787,13 +787,6 @@ public abstract class ContainerBase extends
> LifecycleMBeanBase implements Contai
> }
>
>
> -@Override
> -protected void initInternal() throws LifecycleException {
> -reconfigureStartStopExecutor(getStartStopThreads());
> -super.initInternal();
> -}
> -
> -
> private void reconfigureStartStopExecutor(int threads) {
> if (threads == 1) {
> // Use a fake executor
> @@ -819,6 +812,8 @@ public abstract class ContainerBase extends
> LifecycleMBeanBase implements Contai
> @Override
> protected synchronized void startInternal() throws LifecycleException {
>
> +reconfigureStartStopExecutor(getStartStopThreads());
> +
> // Start our subordinate components, if any
> logger = null;
> getLogger();
> @@ -925,6 +920,12 @@ public abstract class ContainerBase extends
> LifecycleMBeanBase implements Contai
> if (cluster instanceof Lifecycle) {
> ((Lifecycle) cluster).stop();
> }
> +
> +// If init fails, this may be null
> +if (startStopExecutor != null) {
> +startStopExecutor.shutdownNow();
> +startStopExecutor = null;
> +}
> }
>
> @Override
> @@ -954,11 +955,6 @@ public abstract class ContainerBase extends
> LifecycleMBeanBase implements Contai
> parent.removeChild(this);
> }
>
> -// If init fails, this may be null
> -if (startStopExecutor != null) {
> -startStopExecutor.shutdownNow();
> -}
> -
> super.destroyInternal();
> }
>
> diff --git a/java/org/apache/catalina/core/StandardServer.java
> b/java/org/apache/catalina/core/StandardServer.java
> index 80b5026fed..a438
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
On Thu, May 4, 2023 at 3:46 PM Mark Thomas wrote: > > On 04/05/2023 14:41, ma...@apache.org wrote: > > This is an automated email from the ASF dual-hosted git repository. > > > > markt pushed a commit to branch main > > in repository https://gitbox.apache.org/repos/asf/tomcat.git > > > > commit 4b097bf2e9075e9e2949ec5aa410cba3c2b85374 > > Author: Mark Thomas > > AuthorDate: Thu May 4 14:41:01 2023 +0100 > > > > Move management of utility executor from init/destroy to start/stop > > My plan is to back-port this to earlier versions once the May releases > have been tagged. Thanks ;) Rémy > Mark > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
On 04/05/2023 14:41, ma...@apache.org wrote: This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git commit 4b097bf2e9075e9e2949ec5aa410cba3c2b85374 Author: Mark Thomas AuthorDate: Thu May 4 14:41:01 2023 +0100 Move management of utility executor from init/destroy to start/stop My plan is to back-port this to earlier versions once the May releases have been tagged. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4b097bf2e9075e9e2949ec5aa410cba3c2b85374
Author: Mark Thomas
AuthorDate: Thu May 4 14:41:01 2023 +0100
Move management of utility executor from init/destroy to start/stop
---
java/org/apache/catalina/connector/Connector.java | 13 +++---
java/org/apache/catalina/core/ContainerBase.java | 20 +++-
java/org/apache/catalina/core/StandardServer.java | 28 +++---
.../apache/catalina/ha/tcp/SimpleTcpCluster.java | 5 +++-
webapps/docs/changelog.xml | 5
5 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/java/org/apache/catalina/connector/Connector.java
b/java/org/apache/catalina/connector/Connector.java
index c9200e20ca..dac7fdd642 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -992,9 +992,6 @@ public class Connector extends LifecycleMBeanBase {
// Initialize adapter
adapter = new CoyoteAdapter(this);
protocolHandler.setAdapter(adapter);
-if (service != null) {
-
protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
-}
// Make sure parseBodyMethodsSet has a default
if (null == parseBodyMethodsSet) {
@@ -1035,6 +1032,11 @@ public class Connector extends LifecycleMBeanBase {
setState(LifecycleState.STARTING);
+// Configure the utility executor before starting the protocol handler
+if (service != null) {
+
protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
+}
+
try {
protocolHandler.start();
} catch (Exception e) {
@@ -1060,6 +1062,11 @@ public class Connector extends LifecycleMBeanBase {
} catch (Exception e) {
throw new
LifecycleException(sm.getString("coyoteConnector.protocolHandlerStopFailed"),
e);
}
+
+// Remove the utility executor once the protocol handler has been
stopped
+if (service != null) {
+protocolHandler.setUtilityExecutor(null);
+}
}
diff --git a/java/org/apache/catalina/core/ContainerBase.java
b/java/org/apache/catalina/core/ContainerBase.java
index 784c9032ef..a7e7c69a4a 100644
--- a/java/org/apache/catalina/core/ContainerBase.java
+++ b/java/org/apache/catalina/core/ContainerBase.java
@@ -787,13 +787,6 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
}
-@Override
-protected void initInternal() throws LifecycleException {
-reconfigureStartStopExecutor(getStartStopThreads());
-super.initInternal();
-}
-
-
private void reconfigureStartStopExecutor(int threads) {
if (threads == 1) {
// Use a fake executor
@@ -819,6 +812,8 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
@Override
protected synchronized void startInternal() throws LifecycleException {
+reconfigureStartStopExecutor(getStartStopThreads());
+
// Start our subordinate components, if any
logger = null;
getLogger();
@@ -925,6 +920,12 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
if (cluster instanceof Lifecycle) {
((Lifecycle) cluster).stop();
}
+
+// If init fails, this may be null
+if (startStopExecutor != null) {
+startStopExecutor.shutdownNow();
+startStopExecutor = null;
+}
}
@Override
@@ -954,11 +955,6 @@ public abstract class ContainerBase extends
LifecycleMBeanBase implements Contai
parent.removeChild(this);
}
-// If init fails, this may be null
-if (startStopExecutor != null) {
-startStopExecutor.shutdownNow();
-}
-
super.destroyInternal();
}
diff --git a/java/org/apache/catalina/core/StandardServer.java
b/java/org/apache/catalina/core/StandardServer.java
index 80b5026fed..a4383f2503 100644
--- a/java/org/apache/catalina/core/StandardServer.java
+++ b/java/org/apache/catalina/core/StandardServer.java
@@ -901,6 +901,12 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
fireLifecycleEvent(CONFIGURE_START_EVENT, null);
setState(LifecycleState.STARTING);
+// Initialize utility executor
+synchronized (utilityExecutorLock) {
+
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+register(utilityExecutor, "type=UtilityExecutor");
+}
+
globalNamingResources.start();
// Start our defined Services
@@ -961,6 +967,14 @@ public final class StandardServer extends
LifecycleMBeanBa
[tomcat] 01/02: Improve locking of utility executor
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 71032f7b2906ef85b6402b986483d9715bd65e63
Author: Mark Thomas
AuthorDate: Thu May 4 11:41:57 2023 +0100
Improve locking of utility executor
Fixes some edge cases around calling setUtilityThreads() during stop
---
java/org/apache/catalina/core/StandardServer.java | 51 +--
1 file changed, 29 insertions(+), 22 deletions(-)
diff --git a/java/org/apache/catalina/core/StandardServer.java
b/java/org/apache/catalina/core/StandardServer.java
index 09a223fa80..80b5026fed 100644
--- a/java/org/apache/catalina/core/StandardServer.java
+++ b/java/org/apache/catalina/core/StandardServer.java
@@ -424,27 +424,30 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
return;
}
this.utilityThreads = utilityThreads;
-if (oldUtilityThreads != utilityThreads && utilityExecutor != null) {
-
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+synchronized (utilityExecutorLock) {
+if (oldUtilityThreads != utilityThreads && utilityExecutor !=
null) {
+
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+}
}
}
+/*
+ * Callers must be holding utilityExecutorLock
+ */
private void reconfigureUtilityExecutor(int threads) {
-synchronized (utilityExecutorLock) {
-// The ScheduledThreadPoolExecutor doesn't use MaximumPoolSize,
only CorePoolSize is available
-if (utilityExecutor != null) {
-utilityExecutor.setCorePoolSize(threads);
-} else {
-ScheduledThreadPoolExecutor scheduledThreadPoolExecutor = new
ScheduledThreadPoolExecutor(threads,
-new TaskThreadFactory("Catalina-utility-",
utilityThreadsAsDaemon, Thread.MIN_PRIORITY));
-scheduledThreadPoolExecutor.setKeepAliveTime(10,
TimeUnit.SECONDS);
-scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
-
scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
-utilityExecutor = scheduledThreadPoolExecutor;
-utilityExecutorWrapper = new
org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor(
-utilityExecutor);
-}
+// The ScheduledThreadPoolExecutor doesn't use MaximumPoolSize, only
CorePoolSize is available
+if (utilityExecutor != null) {
+utilityExecutor.setCorePoolSize(threads);
+} else {
+ScheduledThreadPoolExecutor scheduledThreadPoolExecutor = new
ScheduledThreadPoolExecutor(threads,
+new TaskThreadFactory("Catalina-utility-",
utilityThreadsAsDaemon, Thread.MIN_PRIORITY));
+scheduledThreadPoolExecutor.setKeepAliveTime(10, TimeUnit.SECONDS);
+scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
+
scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
+utilityExecutor = scheduledThreadPoolExecutor;
+utilityExecutorWrapper = new
org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor(
+utilityExecutor);
}
}
@@ -973,8 +976,10 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
super.initInternal();
// Initialize utility executor
-reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
-register(utilityExecutor, "type=UtilityExecutor");
+synchronized (utilityExecutorLock) {
+
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+register(utilityExecutor, "type=UtilityExecutor");
+}
// Register global String cache
// Note although the cache is global, if there are multiple Servers
@@ -1009,10 +1014,12 @@ public final class StandardServer extends
LifecycleMBeanBase implements Server {
unregister(onameStringCache);
-if (utilityExecutor != null) {
-utilityExecutor.shutdownNow();
-unregister("type=UtilityExecutor");
-utilityExecutor = null;
+synchronized (utilityExecutorLock) {
+if (utilityExecutor != null) {
+utilityExecutor.shutdownNow();
+unregister("type=UtilityExecutor");
+utilityExecutor = null;
+}
}
super.destroyInternal();
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Utility Executor
On Fri, Apr 28, 2023 at 3:35 PM Konstantin Kolinko
wrote:
>
> пт, 28 апр. 2023 г. в 16:21, Konstantin Kolinko :
> >
> > чт, 27 апр. 2023 г. в 19:34, Mark Thomas :
> > >
> > > Hi all,
> > >
> > > As part of a discussion around a Spring Boot issue [1], the question has
> > > been raised whether there is merit in moving the Utility executor
> > > start/stop from StandardServer init/destroy to start/stop.
> > >
> > > I've looked at the code and I don't see any uses of the Executor until
> > > sub-components are in the start phase (there is a little copying of
> > > references that might need to move) so I think the change is doable.
> > >
> > > The main advantage is that in the embedded scenario where there might be
> > > a long series of start / stop / start / stop etc, shutting down the
> > > executor on stop should avoid issues where executor tasks are not
> > > shutdown correctly. My brief code review suggested that Tomcat does this
> > > correctly but the executor is also exposed to application code.
> > >
> > > Thoughts?
> > >
> > > Mark
> > >
> > > [1] https://github.com/spring-projects/spring-boot/issues/34955
> >
> > +1.
> >
> > It sounds reasonable, and thinking about Tomcat being started by
> > Commons Daemon jsvc, I think that init() should not be used to start
> > utility threads.
> >
> > If it causes a regression (I mean, if something really needs to be
> > done at init() time), one may implement a Listener.
>
> Looking at when a Connector starts its pool of threads,
>
> o.a.c.connector.Connector.initInternal() does
>
> // Initialize adapter
> adapter = new CoyoteAdapter(this);
> protocolHandler.setAdapter(adapter);
> if (service != null) {
>
> protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
> }
>
> So it also needs a change.
I noted down that item already (simply looking at all the
getUtilityExecutor calls, basically), it's simply that the pool is
available in init, so it's set up in the endpoint at that time because
"why not", but it's not actually used yet.
Rémy
>
> Best regards,
> Konstantin Kolinko
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Utility Executor
пт, 28 апр. 2023 г. в 16:21, Konstantin Kolinko :
>
> чт, 27 апр. 2023 г. в 19:34, Mark Thomas :
> >
> > Hi all,
> >
> > As part of a discussion around a Spring Boot issue [1], the question has
> > been raised whether there is merit in moving the Utility executor
> > start/stop from StandardServer init/destroy to start/stop.
> >
> > I've looked at the code and I don't see any uses of the Executor until
> > sub-components are in the start phase (there is a little copying of
> > references that might need to move) so I think the change is doable.
> >
> > The main advantage is that in the embedded scenario where there might be
> > a long series of start / stop / start / stop etc, shutting down the
> > executor on stop should avoid issues where executor tasks are not
> > shutdown correctly. My brief code review suggested that Tomcat does this
> > correctly but the executor is also exposed to application code.
> >
> > Thoughts?
> >
> > Mark
> >
> > [1] https://github.com/spring-projects/spring-boot/issues/34955
>
> +1.
>
> It sounds reasonable, and thinking about Tomcat being started by
> Commons Daemon jsvc, I think that init() should not be used to start
> utility threads.
>
> If it causes a regression (I mean, if something really needs to be
> done at init() time), one may implement a Listener.
Looking at when a Connector starts its pool of threads,
o.a.c.connector.Connector.initInternal() does
// Initialize adapter
adapter = new CoyoteAdapter(this);
protocolHandler.setAdapter(adapter);
if (service != null) {
protocolHandler.setUtilityExecutor(service.getServer().getUtilityExecutor());
}
So it also needs a change.
Best regards,
Konstantin Kolinko
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Utility Executor
чт, 27 апр. 2023 г. в 19:34, Mark Thomas : > > Hi all, > > As part of a discussion around a Spring Boot issue [1], the question has > been raised whether there is merit in moving the Utility executor > start/stop from StandardServer init/destroy to start/stop. > > I've looked at the code and I don't see any uses of the Executor until > sub-components are in the start phase (there is a little copying of > references that might need to move) so I think the change is doable. > > The main advantage is that in the embedded scenario where there might be > a long series of start / stop / start / stop etc, shutting down the > executor on stop should avoid issues where executor tasks are not > shutdown correctly. My brief code review suggested that Tomcat does this > correctly but the executor is also exposed to application code. > > Thoughts? > > Mark > > [1] https://github.com/spring-projects/spring-boot/issues/34955 +1. It sounds reasonable, and thinking about Tomcat being started by Commons Daemon jsvc, I think that init() should not be used to start utility threads. If it causes a regression (I mean, if something really needs to be done at init() time), one may implement a Listener. Best regards, Konstantin Kolinko - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Utility Executor
Hi Team,
Its quite awesome discussion to easy the tomcat as proc
I will say it's background compatible since then we added background active
element
StartStop both are in the hand of separate module
Spring Boot is already doing this as tomcat working as in background so it
will boost tomcat will gain the full moment of both are added
Regards
Koti
On Fri, 28 Apr 2023, 15:25 Rémy Maucherat, wrote:
> On Fri, Apr 28, 2023 at 9:02 AM Romain Manni-Bucau
> wrote:
> >
> > Le ven. 28 avr. 2023 à 08:36, Rémy Maucherat a écrit :
> >
> > > On Fri, Apr 28, 2023 at 8:10 AM Romain Manni-Bucau
> > > wrote:
> > > >
> > > > Hi
> > > >
> > > > Guess this utility should be there very early so before other
> > > starts/stops
> > > > sounds good but ultimately init/destroy is better since it avoids to
> > > create
> > > > custom utility threads in subcomponents init/destroy (more destroy
> for
> > > real
> > > > cases I think, not sure for init).
> > >
> > > The Server starts first and stops last, so it seems it will be fine. I
> > > don't quite see why it is a good idea to do utility tasks in init or
> > > destroyed (they should happen in start or stop).
> > > Right now the only item which needs to change is that the connector
> > > endpoint gets it on init. The other components were using it on start.
> > >
> >
> > Still in the spirit of being "stable" in terms of threads and avoiding to
> > create instances per components (which makes the machine under stress, in
> > particular when combined with colocalised deployments - k8s style or just
> > multiple processes locally.
> > The destroy usage I saw was mainly to push (broadcast) metrics captured
> > between stop and destroy by valves - but can ack it is a bit particular
> so
> > can not need something specific.
>
> We are talking about a Server.stop/start here, this is a very
> expensive process (TLS configs, and so on), and I don't understand why
> it would be a good idea to allow background tasks of subcomponents to
> keep running.
>
> > That said I'm not sure it fixes the spring-boot issue at all.
>
> Remy
>
> >
> > >
> > > Remy
> > >
> > > > What about getting it injected from the context and ignoring its
> > > lifecycle
> > > > ("external" notion)? Will keep it globally usable and integrate
> smoothly
> > > > with any env.
> > > >
> > > >
> > > > Le ven. 28 avr. 2023 à 04:27, Han Li a écrit :
> > > >
> > > > >
> > > > >
> > > > > > On Apr 28, 2023, at 00:33, Mark Thomas wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > As part of a discussion around a Spring Boot issue [1], the
> question
> > > has
> > > > > been raised whether there is merit in moving the Utility executor
> > > > > start/stop from StandardServer init/destroy to start/stop.
> > > > > >
> > > > > > I've looked at the code and I don't see any uses of the Executor
> > > until
> > > > > sub-components are in the start phase (there is a little copying of
> > > > > references that might need to move) so I think the change is
> doable.
> > > > >
> > > > > Maybe ContainerBase#startStopExecutor also need same operation
> above?
> > > > >
> > > > > >
> > > > > > The main advantage is that in the embedded scenario where there
> > > might be
> > > > > a long series of start / stop / start / stop etc, shutting down the
> > > > > executor on stop should avoid issues where executor tasks are not
> > > shutdown
> > > > > correctly. My brief code review suggested that Tomcat does this
> > > correctly
> > > > > but the executor is also exposed to application code.
> > > > > >
> > > > > > Thoughts?
> > > > > + 1
> > > > >
> > > > > >
> > > > > > Mark
> > > > > >
> > > > > > [1] https://github.com/spring-projects/spring-boot/issues/34955
> > > > > >
> > > > > >
> -
> > > > > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > > > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > > > > >
> > > > >
> > > > >
> > > > >
> -
> > > > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > > > >
> > > > >
> > >
> > > -
> > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > >
> > >
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>
Re: Utility Executor
Le ven. 28 avr. 2023 à 11:55, Rémy Maucherat a écrit :
> On Fri, Apr 28, 2023 at 9:02 AM Romain Manni-Bucau
> wrote:
> >
> > Le ven. 28 avr. 2023 à 08:36, Rémy Maucherat a écrit :
> >
> > > On Fri, Apr 28, 2023 at 8:10 AM Romain Manni-Bucau
> > > wrote:
> > > >
> > > > Hi
> > > >
> > > > Guess this utility should be there very early so before other
> > > starts/stops
> > > > sounds good but ultimately init/destroy is better since it avoids to
> > > create
> > > > custom utility threads in subcomponents init/destroy (more destroy
> for
> > > real
> > > > cases I think, not sure for init).
> > >
> > > The Server starts first and stops last, so it seems it will be fine. I
> > > don't quite see why it is a good idea to do utility tasks in init or
> > > destroyed (they should happen in start or stop).
> > > Right now the only item which needs to change is that the connector
> > > endpoint gets it on init. The other components were using it on start.
> > >
> >
> > Still in the spirit of being "stable" in terms of threads and avoiding to
> > create instances per components (which makes the machine under stress, in
> > particular when combined with colocalised deployments - k8s style or just
> > multiple processes locally.
> > The destroy usage I saw was mainly to push (broadcast) metrics captured
> > between stop and destroy by valves - but can ack it is a bit particular
> so
> > can not need something specific.
>
> We are talking about a Server.stop/start here, this is a very
> expensive process (TLS configs, and so on), and I don't understand why
> it would be a good idea to allow background tasks of subcomponents to
> keep running.
>
Server means more or less all the subgraph, it is not about leaking tasks
but dont break the backward compat since today it is allowed.
>
> > That said I'm not sure it fixes the spring-boot issue at all.
>
> Remy
>
> >
> > >
> > > Remy
> > >
> > > > What about getting it injected from the context and ignoring its
> > > lifecycle
> > > > ("external" notion)? Will keep it globally usable and integrate
> smoothly
> > > > with any env.
> > > >
> > > >
> > > > Le ven. 28 avr. 2023 à 04:27, Han Li a écrit :
> > > >
> > > > >
> > > > >
> > > > > > On Apr 28, 2023, at 00:33, Mark Thomas wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > As part of a discussion around a Spring Boot issue [1], the
> question
> > > has
> > > > > been raised whether there is merit in moving the Utility executor
> > > > > start/stop from StandardServer init/destroy to start/stop.
> > > > > >
> > > > > > I've looked at the code and I don't see any uses of the Executor
> > > until
> > > > > sub-components are in the start phase (there is a little copying of
> > > > > references that might need to move) so I think the change is
> doable.
> > > > >
> > > > > Maybe ContainerBase#startStopExecutor also need same operation
> above?
> > > > >
> > > > > >
> > > > > > The main advantage is that in the embedded scenario where there
> > > might be
> > > > > a long series of start / stop / start / stop etc, shutting down the
> > > > > executor on stop should avoid issues where executor tasks are not
> > > shutdown
> > > > > correctly. My brief code review suggested that Tomcat does this
> > > correctly
> > > > > but the executor is also exposed to application code.
> > > > > >
> > > > > > Thoughts?
> > > > > + 1
> > > > >
> > > > > >
> > > > > > Mark
> > > > > >
> > > > > > [1] https://github.com/spring-projects/spring-boot/issues/34955
> > > > > >
> > > > > >
> -
> > > > > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > > > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > > > > >
> > > > >
> > > > >
> > > > >
> -
> > > > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > > > >
> > > > >
> > >
> > > -
> > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > >
> > >
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>
Re: Utility Executor
On Fri, Apr 28, 2023 at 9:02 AM Romain Manni-Bucau
wrote:
>
> Le ven. 28 avr. 2023 à 08:36, Rémy Maucherat a écrit :
>
> > On Fri, Apr 28, 2023 at 8:10 AM Romain Manni-Bucau
> > wrote:
> > >
> > > Hi
> > >
> > > Guess this utility should be there very early so before other
> > starts/stops
> > > sounds good but ultimately init/destroy is better since it avoids to
> > create
> > > custom utility threads in subcomponents init/destroy (more destroy for
> > real
> > > cases I think, not sure for init).
> >
> > The Server starts first and stops last, so it seems it will be fine. I
> > don't quite see why it is a good idea to do utility tasks in init or
> > destroyed (they should happen in start or stop).
> > Right now the only item which needs to change is that the connector
> > endpoint gets it on init. The other components were using it on start.
> >
>
> Still in the spirit of being "stable" in terms of threads and avoiding to
> create instances per components (which makes the machine under stress, in
> particular when combined with colocalised deployments - k8s style or just
> multiple processes locally.
> The destroy usage I saw was mainly to push (broadcast) metrics captured
> between stop and destroy by valves - but can ack it is a bit particular so
> can not need something specific.
We are talking about a Server.stop/start here, this is a very
expensive process (TLS configs, and so on), and I don't understand why
it would be a good idea to allow background tasks of subcomponents to
keep running.
> That said I'm not sure it fixes the spring-boot issue at all.
Remy
>
> >
> > Remy
> >
> > > What about getting it injected from the context and ignoring its
> > lifecycle
> > > ("external" notion)? Will keep it globally usable and integrate smoothly
> > > with any env.
> > >
> > >
> > > Le ven. 28 avr. 2023 à 04:27, Han Li a écrit :
> > >
> > > >
> > > >
> > > > > On Apr 28, 2023, at 00:33, Mark Thomas wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > As part of a discussion around a Spring Boot issue [1], the question
> > has
> > > > been raised whether there is merit in moving the Utility executor
> > > > start/stop from StandardServer init/destroy to start/stop.
> > > > >
> > > > > I've looked at the code and I don't see any uses of the Executor
> > until
> > > > sub-components are in the start phase (there is a little copying of
> > > > references that might need to move) so I think the change is doable.
> > > >
> > > > Maybe ContainerBase#startStopExecutor also need same operation above?
> > > >
> > > > >
> > > > > The main advantage is that in the embedded scenario where there
> > might be
> > > > a long series of start / stop / start / stop etc, shutting down the
> > > > executor on stop should avoid issues where executor tasks are not
> > shutdown
> > > > correctly. My brief code review suggested that Tomcat does this
> > correctly
> > > > but the executor is also exposed to application code.
> > > > >
> > > > > Thoughts?
> > > > + 1
> > > >
> > > > >
> > > > > Mark
> > > > >
> > > > > [1] https://github.com/spring-projects/spring-boot/issues/34955
> > > > >
> > > > > -
> > > > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > > > >
> > > >
> > > >
> > > > -
> > > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > > >
> > > >
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> >
> >
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Utility Executor
Le ven. 28 avr. 2023 à 08:36, Rémy Maucherat a écrit :
> On Fri, Apr 28, 2023 at 8:10 AM Romain Manni-Bucau
> wrote:
> >
> > Hi
> >
> > Guess this utility should be there very early so before other
> starts/stops
> > sounds good but ultimately init/destroy is better since it avoids to
> create
> > custom utility threads in subcomponents init/destroy (more destroy for
> real
> > cases I think, not sure for init).
>
> The Server starts first and stops last, so it seems it will be fine. I
> don't quite see why it is a good idea to do utility tasks in init or
> destroyed (they should happen in start or stop).
> Right now the only item which needs to change is that the connector
> endpoint gets it on init. The other components were using it on start.
>
Still in the spirit of being "stable" in terms of threads and avoiding to
create instances per components (which makes the machine under stress, in
particular when combined with colocalised deployments - k8s style or just
multiple processes locally.
The destroy usage I saw was mainly to push (broadcast) metrics captured
between stop and destroy by valves - but can ack it is a bit particular so
can not need something specific.
That said I'm not sure it fixes the spring-boot issue at all.
>
> Remy
>
> > What about getting it injected from the context and ignoring its
> lifecycle
> > ("external" notion)? Will keep it globally usable and integrate smoothly
> > with any env.
> >
> >
> > Le ven. 28 avr. 2023 à 04:27, Han Li a écrit :
> >
> > >
> > >
> > > > On Apr 28, 2023, at 00:33, Mark Thomas wrote:
> > > >
> > > > Hi all,
> > > >
> > > > As part of a discussion around a Spring Boot issue [1], the question
> has
> > > been raised whether there is merit in moving the Utility executor
> > > start/stop from StandardServer init/destroy to start/stop.
> > > >
> > > > I've looked at the code and I don't see any uses of the Executor
> until
> > > sub-components are in the start phase (there is a little copying of
> > > references that might need to move) so I think the change is doable.
> > >
> > > Maybe ContainerBase#startStopExecutor also need same operation above?
> > >
> > > >
> > > > The main advantage is that in the embedded scenario where there
> might be
> > > a long series of start / stop / start / stop etc, shutting down the
> > > executor on stop should avoid issues where executor tasks are not
> shutdown
> > > correctly. My brief code review suggested that Tomcat does this
> correctly
> > > but the executor is also exposed to application code.
> > > >
> > > > Thoughts?
> > > + 1
> > >
> > > >
> > > > Mark
> > > >
> > > > [1] https://github.com/spring-projects/spring-boot/issues/34955
> > > >
> > > > -
> > > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > > >
> > >
> > >
> > > -
> > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > >
> > >
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>
Re: Utility Executor
> On Apr 28, 2023, at 14:35, Rémy Maucherat wrote:
>
> On Fri, Apr 28, 2023 at 8:10 AM Romain Manni-Bucau
> mailto:rmannibu...@gmail.com>> wrote:
>>
>> Hi
>>
>> Guess this utility should be there very early so before other starts/stops
>> sounds good but ultimately init/destroy is better since it avoids to create
>> custom utility threads in subcomponents init/destroy (more destroy for real
>> cases I think, not sure for init).
>
> The Server starts first and stops last, so it seems it will be fine. I
> don't quite see why it is a good idea to do utility tasks in init or
> destroyed (they should happen in start or stop).
> Right now the only item which needs to change is that the connector
> endpoint gets it on init.
Not only connector, also include ContainerBase and SimpleTcpCluster which also
too.
Although I am not familiar with the initialization of SimpleTcpCluster, the
code exists in init method ;)
Han
> The other components were using it on start.
>
> Remy
>
>> What about getting it injected from the context and ignoring its lifecycle
>> ("external" notion)? Will keep it globally usable and integrate smoothly
>> with any env.
>>
>>
>> Le ven. 28 avr. 2023 à 04:27, Han Li a écrit :
>>
>>>
>>>
>>>> On Apr 28, 2023, at 00:33, Mark Thomas wrote:
>>>>
>>>> Hi all,
>>>>
>>>> As part of a discussion around a Spring Boot issue [1], the question has
>>> been raised whether there is merit in moving the Utility executor
>>> start/stop from StandardServer init/destroy to start/stop.
>>>>
>>>> I've looked at the code and I don't see any uses of the Executor until
>>> sub-components are in the start phase (there is a little copying of
>>> references that might need to move) so I think the change is doable.
>>>
>>> Maybe ContainerBase#startStopExecutor also need same operation above?
>>>
>>>>
>>>> The main advantage is that in the embedded scenario where there might be
>>> a long series of start / stop / start / stop etc, shutting down the
>>> executor on stop should avoid issues where executor tasks are not shutdown
>>> correctly. My brief code review suggested that Tomcat does this correctly
>>> but the executor is also exposed to application code.
>>>>
>>>> Thoughts?
>>> + 1
>>>
>>>>
>>>> Mark
>>>>
>>>> [1] https://github.com/spring-projects/spring-boot/issues/34955
>>>>
>>>> -
>>>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>>>> For additional commands, e-mail: dev-h...@tomcat.apache.org
>>>>
>>>
>>>
>>> -
>>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>>> For additional commands, e-mail: dev-h...@tomcat.apache.org
>>>
>>>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> <mailto:dev-unsubscr...@tomcat.apache.org>
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> <mailto:dev-h...@tomcat.apache.org>
Re: Utility Executor
On Fri, Apr 28, 2023 at 8:10 AM Romain Manni-Bucau
wrote:
>
> Hi
>
> Guess this utility should be there very early so before other starts/stops
> sounds good but ultimately init/destroy is better since it avoids to create
> custom utility threads in subcomponents init/destroy (more destroy for real
> cases I think, not sure for init).
The Server starts first and stops last, so it seems it will be fine. I
don't quite see why it is a good idea to do utility tasks in init or
destroyed (they should happen in start or stop).
Right now the only item which needs to change is that the connector
endpoint gets it on init. The other components were using it on start.
Remy
> What about getting it injected from the context and ignoring its lifecycle
> ("external" notion)? Will keep it globally usable and integrate smoothly
> with any env.
>
>
> Le ven. 28 avr. 2023 à 04:27, Han Li a écrit :
>
> >
> >
> > > On Apr 28, 2023, at 00:33, Mark Thomas wrote:
> > >
> > > Hi all,
> > >
> > > As part of a discussion around a Spring Boot issue [1], the question has
> > been raised whether there is merit in moving the Utility executor
> > start/stop from StandardServer init/destroy to start/stop.
> > >
> > > I've looked at the code and I don't see any uses of the Executor until
> > sub-components are in the start phase (there is a little copying of
> > references that might need to move) so I think the change is doable.
> >
> > Maybe ContainerBase#startStopExecutor also need same operation above?
> >
> > >
> > > The main advantage is that in the embedded scenario where there might be
> > a long series of start / stop / start / stop etc, shutting down the
> > executor on stop should avoid issues where executor tasks are not shutdown
> > correctly. My brief code review suggested that Tomcat does this correctly
> > but the executor is also exposed to application code.
> > >
> > > Thoughts?
> > + 1
> >
> > >
> > > Mark
> > >
> > > [1] https://github.com/spring-projects/spring-boot/issues/34955
> > >
> > > -
> > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > >
> >
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> >
> >
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Utility Executor
Hi
Guess this utility should be there very early so before other starts/stops
sounds good but ultimately init/destroy is better since it avoids to create
custom utility threads in subcomponents init/destroy (more destroy for real
cases I think, not sure for init).
What about getting it injected from the context and ignoring its lifecycle
("external" notion)? Will keep it globally usable and integrate smoothly
with any env.
Le ven. 28 avr. 2023 à 04:27, Han Li a écrit :
>
>
> > On Apr 28, 2023, at 00:33, Mark Thomas wrote:
> >
> > Hi all,
> >
> > As part of a discussion around a Spring Boot issue [1], the question has
> been raised whether there is merit in moving the Utility executor
> start/stop from StandardServer init/destroy to start/stop.
> >
> > I've looked at the code and I don't see any uses of the Executor until
> sub-components are in the start phase (there is a little copying of
> references that might need to move) so I think the change is doable.
>
> Maybe ContainerBase#startStopExecutor also need same operation above?
>
> >
> > The main advantage is that in the embedded scenario where there might be
> a long series of start / stop / start / stop etc, shutting down the
> executor on stop should avoid issues where executor tasks are not shutdown
> correctly. My brief code review suggested that Tomcat does this correctly
> but the executor is also exposed to application code.
> >
> > Thoughts?
> + 1
>
> >
> > Mark
> >
> > [1] https://github.com/spring-projects/spring-boot/issues/34955
> >
> > -
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> >
>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>
Re: Utility Executor
> On Apr 28, 2023, at 00:33, Mark Thomas wrote: > > Hi all, > > As part of a discussion around a Spring Boot issue [1], the question has been > raised whether there is merit in moving the Utility executor start/stop from > StandardServer init/destroy to start/stop. > > I've looked at the code and I don't see any uses of the Executor until > sub-components are in the start phase (there is a little copying of > references that might need to move) so I think the change is doable. Maybe ContainerBase#startStopExecutor also need same operation above? > > The main advantage is that in the embedded scenario where there might be a > long series of start / stop / start / stop etc, shutting down the executor on > stop should avoid issues where executor tasks are not shutdown correctly. My > brief code review suggested that Tomcat does this correctly but the executor > is also exposed to application code. > > Thoughts? + 1 > > Mark > > [1] https://github.com/spring-projects/spring-boot/issues/34955 > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Utility Executor
On Thu, Apr 27, 2023 at 6:34 PM Mark Thomas wrote: > > Hi all, > > As part of a discussion around a Spring Boot issue [1], the question has > been raised whether there is merit in moving the Utility executor > start/stop from StandardServer init/destroy to start/stop. > > I've looked at the code and I don't see any uses of the Executor until > sub-components are in the start phase (there is a little copying of > references that might need to move) so I think the change is doable. > > The main advantage is that in the embedded scenario where there might be > a long series of start / stop / start / stop etc, shutting down the > executor on stop should avoid issues where executor tasks are not > shutdown correctly. My brief code review suggested that Tomcat does this > correctly but the executor is also exposed to application code. > > Thoughts? I think I put it here in case there would be utility tasks to do on init, but that's probably not a good design. So you can try to move it, and see if something breaks ;) Rémy > > Mark > > [1] https://github.com/spring-projects/spring-boot/issues/34955 > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Utility Executor
Hi all, As part of a discussion around a Spring Boot issue [1], the question has been raised whether there is merit in moving the Utility executor start/stop from StandardServer init/destroy to start/stop. I've looked at the code and I don't see any uses of the Executor until sub-components are in the start phase (there is a little copying of references that might need to move) so I think the change is doable. The main advantage is that in the embedded scenario where there might be a long series of start / stop / start / stop etc, shutting down the executor on stop should avoid issues where executor tasks are not shutdown correctly. My brief code review suggested that Tomcat does this correctly but the executor is also exposed to application code. Thoughts? Mark [1] https://github.com/spring-projects/spring-boot/issues/34955 - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Expose utility executor to webapps
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 4e3972d33d Expose utility executor to webapps
4e3972d33d is described below
commit 4e3972d33d38c14e8de26240f4d13675273c4e81
Author: remm
AuthorDate: Tue Apr 11 10:10:04 2023 +0200
Expose utility executor to webapps
Part of the discussions in PR#607
Only enabled when the security manager is not active.
---
java/org/apache/catalina/core/StandardContext.java | 7 +++
webapps/docs/changelog.xml | 5 +
2 files changed, 12 insertions(+)
diff --git a/java/org/apache/catalina/core/StandardContext.java
b/java/org/apache/catalina/core/StandardContext.java
index 6a7a03bcb0..5dbeae925c 100644
--- a/java/org/apache/catalina/core/StandardContext.java
+++ b/java/org/apache/catalina/core/StandardContext.java
@@ -135,6 +135,7 @@ import org.apache.tomcat.util.http.Rfc6265CookieProcessor;
import org.apache.tomcat.util.scan.StandardJarScanner;
import org.apache.tomcat.util.security.PrivilegedGetTccl;
import org.apache.tomcat.util.security.PrivilegedSetTccl;
+import org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor;
/**
* Standard implementation of the Context interface. Each child
container must be a Wrapper implementation to
@@ -4918,6 +4919,12 @@ public class StandardContext extends ContainerBase
implements Context, Notificat
// Make the version info available
getServletContext().setAttribute(Globals.WEBAPP_VERSION,
getWebappVersion());
+
+// Make the utility executor available
+if (!Globals.IS_SECURITY_ENABLED) {
+
getServletContext().setAttribute(ScheduledThreadPoolExecutor.class.getName(),
+
Container.getService(this).getServer().getUtilityExecutor());
+}
}
// Set up the context init params
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 60626d2702..7d4dd421f0 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -175,6 +175,11 @@
avoid possible JVM thread creation in the webapp context on some
platforms. (remm)
+
+Make the server utility executor available to webapps using a Servlet
+context attribute named
+
org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor. (remm)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 10.1.x updated: Expose utility executor to webapps
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new fe797d1417 Expose utility executor to webapps
fe797d1417 is described below
commit fe797d141723bd088825707b85b6c8abc957d012
Author: remm
AuthorDate: Tue Apr 11 10:10:04 2023 +0200
Expose utility executor to webapps
Part of the discussions in PR#607
Only enabled when the security manager is not active.
---
java/org/apache/catalina/core/StandardContext.java | 7 +++
webapps/docs/changelog.xml | 5 +
2 files changed, 12 insertions(+)
diff --git a/java/org/apache/catalina/core/StandardContext.java
b/java/org/apache/catalina/core/StandardContext.java
index 4e6af40ba0..34bbc4ac2d 100644
--- a/java/org/apache/catalina/core/StandardContext.java
+++ b/java/org/apache/catalina/core/StandardContext.java
@@ -134,6 +134,7 @@ import org.apache.tomcat.util.http.Rfc6265CookieProcessor;
import org.apache.tomcat.util.scan.StandardJarScanner;
import org.apache.tomcat.util.security.PrivilegedGetTccl;
import org.apache.tomcat.util.security.PrivilegedSetTccl;
+import org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor;
/**
* Standard implementation of the Context interface. Each child
container must be a Wrapper implementation to
@@ -4864,6 +4865,12 @@ public class StandardContext extends ContainerBase
implements Context, Notificat
// Make the version info available
getServletContext().setAttribute(Globals.WEBAPP_VERSION,
getWebappVersion());
+
+// Make the utility executor available
+if (!Globals.IS_SECURITY_ENABLED) {
+
getServletContext().setAttribute(ScheduledThreadPoolExecutor.class.getName(),
+
Container.getService(this).getServer().getUtilityExecutor());
+}
}
// Set up the context init params
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index e084cbd1c3..b5792a67e3 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -175,6 +175,11 @@
avoid possible JVM thread creation in the webapp context on some
platforms. (remm)
+
+Make the server utility executor available to webapps using a Servlet
+context attribute named
+
org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor. (remm)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat] branch master updated: Use utility executor for scanning
On Fri, Sep 18, 2020 at 12:06 PM Mark Thomas wrote:
> On 18/09/2020 10:40, r...@apache.org wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > remm pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> > new 27bb36b Use utility executor for scanning
> > 27bb36b is described below
> >
> > commit 27bb36bdaa56d62fdcc28d3b8e3d4c25f4a44a50
> > Author: remm
> > AuthorDate: Fri Sep 18 11:40:17 2020 +0200
> >
> > Use utility executor for scanning
> >
> > PR354 submitted by Jatin Kamnani.
> > The flag in on the context, as that's where it belongs. Defaults to
> > false, can be configured through the context defaults or per context.
>
>
>
> > +protected void processAnnotationsInParallel(Set fragments,
> boolean handlesTypesOnly,
> > +Map JavaClassCacheEntry> javaClassCache) {
> > +Server s = getServer();
> > +ExecutorService pool = null;
> > +try {
> > +pool = s.getUtilityExecutor();
> > +List> futures = new ArrayList<>(fragments.size());
> > +for (WebXml fragment : fragments) {
> > +Runnable task = new AnnotationScanTask(fragment,
> handlesTypesOnly, javaClassCache);
> > +futures.add(pool.submit(task));
> > +}
> > +try {
> > +for (Future future : futures) {
> > +future.get();
> > +}
> > +} catch (Exception e) {
> > +throw new
> RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"),
> e);
> > +}
> > +} finally {
> > +if (pool != null) {
> > +pool.shutdownNow();
> > +}
> > }
> > }
>
> Why is the executor being shutdown here? What about other Tomcat
> components that might be using it or want to use it in the future?
>
I forgot to remove that. It does nothing since there's a facade for the
executor which does a noop there.
Rémy
Re: [tomcat] branch master updated: Use utility executor for scanning
On 18/09/2020 10:40, r...@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> remm pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new 27bb36b Use utility executor for scanning
> 27bb36b is described below
>
> commit 27bb36bdaa56d62fdcc28d3b8e3d4c25f4a44a50
> Author: remm
> AuthorDate: Fri Sep 18 11:40:17 2020 +0200
>
> Use utility executor for scanning
>
> PR354 submitted by Jatin Kamnani.
> The flag in on the context, as that's where it belongs. Defaults to
> false, can be configured through the context defaults or per context.
> +protected void processAnnotationsInParallel(Set fragments,
> boolean handlesTypesOnly,
> +Map JavaClassCacheEntry> javaClassCache) {
> +Server s = getServer();
> +ExecutorService pool = null;
> +try {
> +pool = s.getUtilityExecutor();
> +List> futures = new ArrayList<>(fragments.size());
> +for (WebXml fragment : fragments) {
> +Runnable task = new AnnotationScanTask(fragment,
> handlesTypesOnly, javaClassCache);
> +futures.add(pool.submit(task));
> +}
> +try {
> +for (Future future : futures) {
> +future.get();
> +}
> +} catch (Exception e) {
> +throw new
> RuntimeException(sm.getString("contextConfig.processAnnotationsInParallelFailure"),
> e);
> +}
> +} finally {
> +if (pool != null) {
> +pool.shutdownNow();
> +}
> }
> }
Why is the executor being shutdown here? What about other Tomcat
components that might be using it or want to use it in the future?
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Use utility executor for scanning
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 101476c Use utility executor for scanning
101476c is described below
commit 101476c3536ac09a105d0b603b2c51dd0f3770e9
Author: remm
AuthorDate: Fri Sep 18 11:40:17 2020 +0200
Use utility executor for scanning
PR354 submitted by Jatin Kamnani.
The flag in on the context, as that's where it belongs. Defaults to
false, can be configured through the context defaults or per context.
---
java/org/apache/catalina/Context.java | 14 +++
java/org/apache/catalina/core/StandardContext.java | 19
.../apache/catalina/core/mbeans-descriptors.xml| 4 +
.../org/apache/catalina/startup/ContextConfig.java | 116 +
.../org/apache/catalina/startup/FailedContext.java | 6 ++
.../catalina/startup/LocalStrings.properties | 1 +
test/org/apache/tomcat/unittest/TesterContext.java | 6 ++
webapps/docs/changelog.xml | 4 +
webapps/docs/config/context.xml| 8 ++
9 files changed, 156 insertions(+), 22 deletions(-)
diff --git a/java/org/apache/catalina/Context.java
b/java/org/apache/catalina/Context.java
index 3e647dc..5aac225 100644
--- a/java/org/apache/catalina/Context.java
+++ b/java/org/apache/catalina/Context.java
@@ -761,6 +761,20 @@ public interface Context extends Container, ContextBind {
public String getContainerSciFilter();
+/**
+ * @return the value of the parallel annotation scanning flag. If true,
+ * it will dispatch scanning to the utility executor.
+ */
+public boolean isParallelAnnotationScanning();
+
+/**
+ * Set the parallel annotation scanning value.
+ *
+ * @param parallelAnnotationScanning new parallel annotation scanning flag
+ */
+public void setParallelAnnotationScanning(boolean
parallelAnnotationScanning);
+
+
// - Public Methods
/**
diff --git a/java/org/apache/catalina/core/StandardContext.java
b/java/org/apache/catalina/core/StandardContext.java
index 12641d7..06f7621 100644
--- a/java/org/apache/catalina/core/StandardContext.java
+++ b/java/org/apache/catalina/core/StandardContext.java
@@ -828,6 +828,8 @@ public class StandardContext extends ContainerBase
private boolean createUploadTargets = false;
+private boolean parallelAnnotationScanning = false;
+
// - Context Properties
@Override
@@ -1404,6 +1406,23 @@ public class StandardContext extends ContainerBase
}
+@Override
+public void setParallelAnnotationScanning(boolean
parallelAnnotationScanning) {
+
+boolean oldParallelAnnotationScanning =
this.parallelAnnotationScanning;
+this.parallelAnnotationScanning = parallelAnnotationScanning;
+support.firePropertyChange("parallelAnnotationScanning",
oldParallelAnnotationScanning,
+this.parallelAnnotationScanning);
+
+}
+
+
+@Override
+public boolean isParallelAnnotationScanning() {
+return this.parallelAnnotationScanning;
+}
+
+
/**
* @return the Locale to character set mapper for this Context.
*/
diff --git a/java/org/apache/catalina/core/mbeans-descriptors.xml
b/java/org/apache/catalina/core/mbeans-descriptors.xml
index 1c0733f..50be99f 100644
--- a/java/org/apache/catalina/core/mbeans-descriptors.xml
+++ b/java/org/apache/catalina/core/mbeans-descriptors.xml
@@ -208,6 +208,10 @@
description="The name of this Context"
type="java.lang.String"/>
+
+
diff --git a/java/org/apache/catalina/startup/ContextConfig.java
b/java/org/apache/catalina/startup/ContextConfig.java
index d7b5a77..3dc1638 100644
--- a/java/org/apache/catalina/startup/ContextConfig.java
+++ b/java/org/apache/catalina/startup/ContextConfig.java
@@ -39,6 +39,8 @@ import java.util.Map.Entry;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
import javax.servlet.MultipartConfigElement;
import javax.servlet.ServletContainerInitializer;
@@ -122,7 +124,6 @@ public class ContextConfig implements LifecycleListener {
private static final Log log = LogFactory.getLog(ContextConfig.class);
-
/**
* The string resources for this package.
*/
@@ -1374,7 +1375,14 @@ public class ContextConfig implements LifecycleListener {
protected void processClasses(WebXml webXml, Set orderedFragments)
{
// Step 4. Process /WEB-INF/classes for annotations and
// @HandlesTypes matches
-Map javaClassCache = new HashMap&l
[tomcat] branch master updated: Use utility executor for scanning
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 27bb36b Use utility executor for scanning
27bb36b is described below
commit 27bb36bdaa56d62fdcc28d3b8e3d4c25f4a44a50
Author: remm
AuthorDate: Fri Sep 18 11:40:17 2020 +0200
Use utility executor for scanning
PR354 submitted by Jatin Kamnani.
The flag in on the context, as that's where it belongs. Defaults to
false, can be configured through the context defaults or per context.
---
java/org/apache/catalina/Context.java | 14 +++
java/org/apache/catalina/core/StandardContext.java | 19
.../apache/catalina/core/mbeans-descriptors.xml| 4 +
.../org/apache/catalina/startup/ContextConfig.java | 116 +
.../org/apache/catalina/startup/FailedContext.java | 5 +
.../catalina/startup/LocalStrings.properties | 1 +
test/org/apache/tomcat/unittest/TesterContext.java | 5 +
webapps/docs/changelog.xml | 4 +
webapps/docs/config/context.xml| 8 ++
9 files changed, 154 insertions(+), 22 deletions(-)
diff --git a/java/org/apache/catalina/Context.java
b/java/org/apache/catalina/Context.java
index b8f55a0..4cf844c 100644
--- a/java/org/apache/catalina/Context.java
+++ b/java/org/apache/catalina/Context.java
@@ -762,6 +762,20 @@ public interface Context extends Container, ContextBind {
public String getContainerSciFilter();
+/**
+ * @return the value of the parallel annotation scanning flag. If true,
+ * it will dispatch scanning to the utility executor.
+ */
+public boolean isParallelAnnotationScanning();
+
+/**
+ * Set the parallel annotation scanning value.
+ *
+ * @param parallelAnnotationScanning new parallel annotation scanning flag
+ */
+public void setParallelAnnotationScanning(boolean
parallelAnnotationScanning);
+
+
// - Public Methods
/**
diff --git a/java/org/apache/catalina/core/StandardContext.java
b/java/org/apache/catalina/core/StandardContext.java
index ce51d6c..a34f14e 100644
--- a/java/org/apache/catalina/core/StandardContext.java
+++ b/java/org/apache/catalina/core/StandardContext.java
@@ -833,6 +833,8 @@ public class StandardContext extends ContainerBase
private boolean dispatcherWrapsSameObject =
Globals.STRICT_SERVLET_COMPLIANCE;
+private boolean parallelAnnotationScanning = false;
+
// - Context Properties
@Override
@@ -1447,6 +1449,23 @@ public class StandardContext extends ContainerBase
}
+@Override
+public void setParallelAnnotationScanning(boolean
parallelAnnotationScanning) {
+
+boolean oldParallelAnnotationScanning =
this.parallelAnnotationScanning;
+this.parallelAnnotationScanning = parallelAnnotationScanning;
+support.firePropertyChange("parallelAnnotationScanning",
oldParallelAnnotationScanning,
+this.parallelAnnotationScanning);
+
+}
+
+
+@Override
+public boolean isParallelAnnotationScanning() {
+return this.parallelAnnotationScanning;
+}
+
+
/**
* @return the Locale to character set mapper for this Context.
*/
diff --git a/java/org/apache/catalina/core/mbeans-descriptors.xml
b/java/org/apache/catalina/core/mbeans-descriptors.xml
index 1c0733f..50be99f 100644
--- a/java/org/apache/catalina/core/mbeans-descriptors.xml
+++ b/java/org/apache/catalina/core/mbeans-descriptors.xml
@@ -208,6 +208,10 @@
description="The name of this Context"
type="java.lang.String"/>
+
+
diff --git a/java/org/apache/catalina/startup/ContextConfig.java
b/java/org/apache/catalina/startup/ContextConfig.java
index c1fe029..a3cfcb8 100644
--- a/java/org/apache/catalina/startup/ContextConfig.java
+++ b/java/org/apache/catalina/startup/ContextConfig.java
@@ -39,6 +39,8 @@ import java.util.Map.Entry;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
import jakarta.servlet.MultipartConfigElement;
import jakarta.servlet.ServletContainerInitializer;
@@ -122,7 +124,6 @@ public class ContextConfig implements LifecycleListener {
private static final Log log = LogFactory.getLog(ContextConfig.class);
-
/**
* The string resources for this package.
*/
@@ -1374,7 +1375,14 @@ public class ContextConfig implements LifecycleListener {
protected void processClasses(WebXml webXml, Set orderedFragments)
{
// Step 4. Process /WEB-INF/classes for annotations and
// @HandlesTypes matches
-
