Bug 60368 - Embedded org.apache.catalina.startup.Tomcat instance can no longer be started without a Connector
Summary: Embedded org.apache.catalina.startup.Tomcat instance can no longer be started...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.5.8
Hardware: PC All
: P2 regression (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-14 09:23 UTC by Andy Wilkinson
Modified: 2016-11-16 16:25 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Wilkinson 2016-11-14 09:23:16 UTC
The changes made in http://svn.apache.org/viewvc?view=revision&revision=1766521 have regressed org.apache.catalina.startup.Tomcat's behaviour at start up.

Prior to those changes it was possible to create a Tomcat instance, remove its default Connector and start it. At this point no Connectors would be started. One or more Connectors could then be added at which point they would be started automatically.

The behaviour prior to the changes made in 1766521 was useful as it allowed precise control over when Tomcat would start accepting and processing requests. As far as I can tell, this is no longer possible.
Comment 1 Remy Maucherat 2016-11-14 10:17:08 UTC
It looks to me you were taking advantage of a hack: getConnector, then remove connector. Uh. I'll restore in 8.5 code so that a default connector is only created once.

In 9, it would be good to remove the automatic connector creation in init and start, since it doesn't make sense.
Comment 2 Remy Maucherat 2016-11-14 14:20:06 UTC
Fixed in Tomcat 8.5.9. In Tomcat 9.0M14, I have removed automatic connector creation on start or when using Tomcat.getConnector.
I left in place the creation of other components such as service/engine/host since the defaults for them are more likely to be acceptable and they are not automatically created on start either.
Comment 3 Andy Wilkinson 2016-11-14 14:59:17 UTC
Thank you. Would it be possible to publish an 8.5 snapshot that contains the fix?
Comment 4 Mark Thomas 2016-11-14 15:03:12 UTC
I can take care of that. Give it an hour or so and the snapshot should be available in the usual location.
Comment 5 Andy Wilkinson 2016-11-14 16:10:17 UTC
Thanks, Mark. I've tried the snapshot and, unfortunately, it hasn't restored the previous behaviour.

The latest change relies on getConnector() being called directly on the Tomcat instance. Our code that works with 8.5.6 doesn't call getConnector(), instead it retrieves the Connectors from their Services: https://github.com/spring-projects/spring-boot/blob/6aa57e8ffe0a868fa5ed081f784015f3463022b3/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java#L138-L146. As a result the new defaultConnectorCreated flag remains false and when start is called the default connector is created when previously it was not.
Comment 6 Remy Maucherat 2016-11-14 16:30:44 UTC
When you write hacks, you get into trouble at some point :) 

As you're not calling setConnector either, I don't understand how start doesn't create a new default connector behind your back by calling getConnector ... Look at the code in Tomcat 8.0.

Solutions:
- The new behavior from Tomcat 9 can be ported (IMO creating default connectors is a bad idea)
- Call getConnector, it won't hurt
Comment 7 Andy Wilkinson 2016-11-14 16:52:19 UTC
> As you're not calling setConnector either

We do call setConnector but I neglected to mention it until now as I hadn't realised it was pertinent. Sorry. It's called as part of configuring the embedded Tomcat instance. This configuration can also include customisations that are made by users' code. Once everything's been configured we temporarily remove the connectors and start Tomcat. Once Tomcat and other bits and pieces have successfully started and we're in a position to start handling requests, the Connectors are reinstated.

Previously (8.5.6 and earlier), calling setConnector meant that the Connector was cached in Tomcat's connector field. This cached Connector would then be returned from getConnector() instead of creating the default Connector which prevented the call to start from creating an unwanted Connector.

> Call getConnector, it won't hurt

Thanks for the suggestion. Won't we end up with the Service having an extra connector (one from our call to setConnector and one from the call to getConnector)?. Instead, perhaps the defaultConnectorCreated flag could be set when setConnector is called?
Comment 8 Remy Maucherat 2016-11-14 17:05:10 UTC
Ok, let's try that, the behavior from 8.0 makes sense if setConnector is called.
Comment 9 Andy Wilkinson 2016-11-15 15:58:36 UTC
Thanks, Remy.

Mark, could you publish another snapshot please?
Comment 10 Violeta Georgieva 2016-11-16 14:50:53 UTC
Hi,

(In reply to Andy Wilkinson from comment #9)
> Mark, could you publish another snapshot please?

I published a new snapshot. Can you test it?

Thanks,
Violeta
Comment 11 Andy Wilkinson 2016-11-16 15:13:19 UTC
Thanks, Violeta. I've tested the -5 snapshot and it hasn't completely fixed the problem.

Our code currently does this:

tomcat.getService().addConnector(connector);
tomcat.setConnector(connector);

This worked fine in 8.5.6 and earlier.

With the latest snapshot we still end up with two connectors because adding the connector to the service prevents setConnector from setting the defaultConnectorCreated flag. So, strictly speaking, there's still a behaviour change here.

That said, as far as I can tell there's no need to be add the Connector to the Service and set it on the Tomcat instance so we can change our code to just do this:

tomcat.setConnector(connector);

It should then work as it did with 8.5.6.
Comment 12 Remy Maucherat 2016-11-16 15:23:41 UTC
I made another change to set the flag in all cases in setConnector.
Comment 13 Violeta Georgieva 2016-11-16 15:27:35 UTC
(In reply to Remy Maucherat from comment #12)
> I made another change to set the flag in all cases in setConnector.

Ok I can publish another snapshot if it is needed?
Comment 14 Violeta Georgieva 2016-11-16 16:08:11 UTC
(In reply to Violeta Georgieva from comment #13)
> (In reply to Remy Maucherat from comment #12)
> > I made another change to set the flag in all cases in setConnector.
> 
> Ok I can publish another snapshot if it is needed?

I published another snapshot.

Regards,
Violeta
Comment 15 Andy Wilkinson 2016-11-16 16:25:30 UTC
Thank you, Remy and Violeta. The behaviour of the latest snapshot (-6) is the same as 8.5.6.