You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2023/05/04 13:41:38 UTC
[tomcat] branch main updated (43643fe02c -> 4b097bf2e9)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from 43643fe02c Add JRE fix detail to workaround so it can be removed later.
new 71032f7b29 Improve locking of utility executor
new 4b097bf2e9 Move management of utility executor from init/destroy to start/stop
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
java/org/apache/catalina/connector/Connector.java | 13 +++--
java/org/apache/catalina/core/ContainerBase.java | 20 +++-----
java/org/apache/catalina/core/StandardServer.java | 59 ++++++++++++----------
.../apache/catalina/ha/tcp/SimpleTcpCluster.java | 5 +-
webapps/docs/changelog.xml | 5 ++
5 files changed, 60 insertions(+), 42 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
Posted by Mark Thomas <ma...@homeinbox.net>.
On 05/05/2023 13:17, Han Li wrote:
>
>
>> On May 5, 2023, at 18:42, Mark Thomas <ma...@apache.org> wrote:
>>
>> On 05/05/2023 04:21, Han Li wrote:
>>>> On May 4, 2023, at 21:41, markt@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 <ma...@apache.org>
>>>> 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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
Posted by Han Li <li...@apache.org>.
> On May 5, 2023, at 18:42, Mark Thomas <ma...@apache.org> wrote:
>
> On 05/05/2023 04:21, Han Li wrote:
>>> On May 4, 2023, at 21:41, markt@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 <ma...@apache.org>
>>> 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-unsubscribe@tomcat.apache.org <ma...@tomcat.apache.org>
> For additional commands, e-mail: dev-help@tomcat.apache.org <ma...@tomcat.apache.org>
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
Posted by Mark Thomas <ma...@apache.org>.
On 05/05/2023 04:21, Han Li wrote:
>
>
>> On May 4, 2023, at 21:41, markt@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 <ma...@apache.org>
>> 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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
Posted by Han Li <li...@apache.org>.
> On May 4, 2023, at 21:41, markt@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 <ma...@apache.org>
> 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..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 LifecycleMBeanBase implements Server {
> service.stop();
> }
>
> + synchronized (utilityExecutorLock) {
> + if (utilityExecutor != null) {
> + utilityExecutor.shutdownNow();
> + unregister("type=UtilityExecutor");
> + utilityExecutor = null;
> + }
> + }
> +
> globalNamingResources.stop();
>
> stopAwait();
> @@ -975,12 +989,6 @@ public final class StandardServer extends LifecycleMBeanBase implements Server {
>
> super.initInternal();
>
> - // Initialize utility executor
> - synchronized (utilityExecutorLock) {
> - reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
> - register(utilityExecutor, "type=UtilityExecutor");
> - }
> -
> // Register global String cache
> // Note although the cache is global, if there are multiple Servers
> // present in the JVM (may happen when embedding) then the same cache
> @@ -1014,14 +1022,6 @@ public final class StandardServer extends LifecycleMBeanBase implements Server {
>
> unregister(onameStringCache);
>
> - synchronized (utilityExecutorLock) {
> - if (utilityExecutor != null) {
> - utilityExecutor.shutdownNow();
> - unregister("type=UtilityExecutor");
> - utilityExecutor = null;
> - }
> - }
> -
> super.destroyInternal();
> }
>
> diff --git a/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java b/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
> index 83b27bddac..3f01ebf0eb 100644
> --- a/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
> +++ b/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
> @@ -530,7 +530,6 @@ public class SimpleTcpCluster extends LifecycleMBeanBase
> name.append(",component=Deployer");
> onameClusterDeployer = register(clusterDeployer, name.toString());
> }
> - channel.setUtilityExecutor(Container.getService(getContainer()).getServer().getUtilityExecutor());
> }
>
>
> @@ -548,6 +547,8 @@ public class SimpleTcpCluster extends LifecycleMBeanBase
> log.info(sm.getString("simpleTcpCluster.start"));
> }
>
> + channel.setUtilityExecutor(Container.getService(getContainer()).getServer().getUtilityExecutor());
> +
> try {
> checkDefaults();
> registerClusterValve();
> @@ -655,6 +656,8 @@ public class SimpleTcpCluster extends LifecycleMBeanBase
> } catch (Exception x) {
> log.error(sm.getString("simpleTcpCluster.stopUnable"), x);
> }
> +
> + channel.setUtilityExecutor(null);
> }
>
>
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index 131799a300..ebdac44cee 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -134,6 +134,11 @@
> file locking protection or the manager servlet. Submitted
> by Jack Shirazi. (remm)
> </fix>
> + <scode>
> + Move the management of the utility executor from the
> + <code>init()</code>/<code>destroy()</code> methods of components to the
> + <code>start()</code>/<code>stop()</code> methods. (markt)
> + </scode>
> </changelog>
> </subsection>
> <subsection name="Coyote">
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
Posted by koteswara Rao Gundapaneni <ko...@gmail.com>.
Hi Team ,
Management of start and stop might be arrested
Regards
Koti
On Thu, 4 May 2023, 19:12 , <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 <ma...@apache.org>
> 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
> LifecycleMBeanBase implements Server {
> service.stop();
> }
>
> + synchronized (utilityExecutorLock) {
> + if (utilityExecutor != null) {
> + utilityExecutor.shutdownNow();
> + unregister("type=UtilityExecutor");
> + utilityExecutor = null;
> + }
> + }
> +
> globalNamingResources.stop();
>
> stopAwait();
> @@ -975,12 +989,6 @@ public final class StandardServer extends
> LifecycleMBeanBase implements Server {
>
> super.initInternal();
>
> - // Initialize utility executor
> - synchronized (utilityExecutorLock) {
> -
> reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
> - register(utilityExecutor, "type=UtilityExecutor");
> - }
> -
> // Register global String cache
> // Note although the cache is global, if there are multiple
> Servers
> // present in the JVM (may happen when embedding) then the same
> cache
> @@ -1014,14 +1022,6 @@ public final class StandardServer extends
> LifecycleMBeanBase implements Server {
>
> unregister(onameStringCache);
>
> - synchronized (utilityExecutorLock) {
> - if (utilityExecutor != null) {
> - utilityExecutor.shutdownNow();
> - unregister("type=UtilityExecutor");
> - utilityExecutor = null;
> - }
> - }
> -
> super.destroyInternal();
> }
>
> diff --git a/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
> b/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
> index 83b27bddac..3f01ebf0eb 100644
> --- a/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
> +++ b/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
> @@ -530,7 +530,6 @@ public class SimpleTcpCluster extends
> LifecycleMBeanBase
> name.append(",component=Deployer");
> onameClusterDeployer = register(clusterDeployer,
> name.toString());
> }
> -
> channel.setUtilityExecutor(Container.getService(getContainer()).getServer().getUtilityExecutor());
> }
>
>
> @@ -548,6 +547,8 @@ public class SimpleTcpCluster extends
> LifecycleMBeanBase
> log.info(sm.getString("simpleTcpCluster.start"));
> }
>
> +
> channel.setUtilityExecutor(Container.getService(getContainer()).getServer().getUtilityExecutor());
> +
> try {
> checkDefaults();
> registerClusterValve();
> @@ -655,6 +656,8 @@ public class SimpleTcpCluster extends
> LifecycleMBeanBase
> } catch (Exception x) {
> log.error(sm.getString("simpleTcpCluster.stopUnable"), x);
> }
> +
> + channel.setUtilityExecutor(null);
> }
>
>
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index 131799a300..ebdac44cee 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -134,6 +134,11 @@
> file locking protection or the manager servlet. Submitted
> by Jack Shirazi. (remm)
> </fix>
> + <scode>
> + Move the management of the utility executor from the
> + <code>init()</code>/<code>destroy()</code> methods of components
> to the
> + <code>start()</code>/<code>stop()</code> methods. (markt)
> + </scode>
> </changelog>
> </subsection>
> <subsection name="Coyote">
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
Posted by Rémy Maucherat <re...@apache.org>.
On Thu, May 4, 2023 at 3:46 PM Mark Thomas <ma...@apache.org> wrote:
>
> On 04/05/2023 14:41, markt@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 <ma...@apache.org>
> > 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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
Posted by Mark Thomas <ma...@apache.org>.
On 04/05/2023 14:41, markt@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 <ma...@apache.org>
> 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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/02: Move management of utility executor from init/destroy to start/stop
Posted by ma...@apache.org.
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 <ma...@apache.org>
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 LifecycleMBeanBase implements Server {
service.stop();
}
+ synchronized (utilityExecutorLock) {
+ if (utilityExecutor != null) {
+ utilityExecutor.shutdownNow();
+ unregister("type=UtilityExecutor");
+ utilityExecutor = null;
+ }
+ }
+
globalNamingResources.stop();
stopAwait();
@@ -975,12 +989,6 @@ public final class StandardServer extends LifecycleMBeanBase implements Server {
super.initInternal();
- // Initialize utility executor
- synchronized (utilityExecutorLock) {
- reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
- register(utilityExecutor, "type=UtilityExecutor");
- }
-
// Register global String cache
// Note although the cache is global, if there are multiple Servers
// present in the JVM (may happen when embedding) then the same cache
@@ -1014,14 +1022,6 @@ public final class StandardServer extends LifecycleMBeanBase implements Server {
unregister(onameStringCache);
- synchronized (utilityExecutorLock) {
- if (utilityExecutor != null) {
- utilityExecutor.shutdownNow();
- unregister("type=UtilityExecutor");
- utilityExecutor = null;
- }
- }
-
super.destroyInternal();
}
diff --git a/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java b/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
index 83b27bddac..3f01ebf0eb 100644
--- a/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
+++ b/java/org/apache/catalina/ha/tcp/SimpleTcpCluster.java
@@ -530,7 +530,6 @@ public class SimpleTcpCluster extends LifecycleMBeanBase
name.append(",component=Deployer");
onameClusterDeployer = register(clusterDeployer, name.toString());
}
- channel.setUtilityExecutor(Container.getService(getContainer()).getServer().getUtilityExecutor());
}
@@ -548,6 +547,8 @@ public class SimpleTcpCluster extends LifecycleMBeanBase
log.info(sm.getString("simpleTcpCluster.start"));
}
+ channel.setUtilityExecutor(Container.getService(getContainer()).getServer().getUtilityExecutor());
+
try {
checkDefaults();
registerClusterValve();
@@ -655,6 +656,8 @@ public class SimpleTcpCluster extends LifecycleMBeanBase
} catch (Exception x) {
log.error(sm.getString("simpleTcpCluster.stopUnable"), x);
}
+
+ channel.setUtilityExecutor(null);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 131799a300..ebdac44cee 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -134,6 +134,11 @@
file locking protection or the manager servlet. Submitted
by Jack Shirazi. (remm)
</fix>
+ <scode>
+ Move the management of the utility executor from the
+ <code>init()</code>/<code>destroy()</code> methods of components to the
+ <code>start()</code>/<code>stop()</code> methods. (markt)
+ </scode>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/02: Improve locking of utility executor
Posted by ma...@apache.org.
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 <ma...@apache.org>
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org