> 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, 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());
>> 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>

Reply via email to