Bug 50360

Summary: Server socket still bound after Embedded.stop is invoked
Product: Tomcat 7 Reporter: Martin Grotzke <martin.grotzke>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 7.0.4   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Attachments: Add setState(LifecycleState.MUST_DESTROY) to Connector.stopInternal (based on 7.0.4)

Description Martin Grotzke 2010-11-28 20:03:18 UTC
After stopping an Embedded tomcat the server socket is still bound (in state LISTEN).

This test case fails:


import java.io.File;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.ServerSocket;
import java.net.UnknownHostException;

import org.apache.catalina.Context;
import org.apache.catalina.Engine;
import org.apache.catalina.Host;
import org.apache.catalina.LifecycleException;
import org.apache.catalina.LifecycleState;
import org.apache.catalina.connector.Connector;
import org.apache.catalina.session.StandardManager;
import org.apache.catalina.startup.Embedded;
import org.testng.Assert;
import org.testng.annotations.Test;


@SuppressWarnings( "deprecation" )
public class TestEmbeddedStop {

    @Test
    public void testShutdown() throws Exception {
        final int port = 18888;
        final Embedded tomcat1 = createCatalina( port );
        tomcat1.start();
        tomcat1.stop();
        try {
            new ServerSocket( port );
        } catch ( final IOException e ) {
            Assert.fail( "Could not open new server socket on port " + port, e );
        }
    }

    public static Embedded createCatalina( final int port ) throws MalformedURLException,
            UnknownHostException, LifecycleException {

        final Embedded catalina = new Embedded() {

            @Override
            // Workaround for #50358 - Embedded.stopInternal sets state to STARTING, should be STOPPING
            protected void stopInternal() throws LifecycleException {
                super.stopInternal();
                setState(LifecycleState.STOPPING);
            }

        };

        final Engine engine = catalina.createEngine();

        engine.setName( "engine-" + port );
        engine.setDefaultHost( "localhost" );

        final String docBase = System.getProperty( "java.io.tmpdir" );
        final Host host = catalina.createHost( "localhost", docBase );
        engine.addChild( host );

        final Context context = catalina.createContext( "", "webapp" );
        host.addChild( context );
        new File( docBase, "webapp" ).mkdirs();

        context.setManager( new StandardManager() );

        catalina.addEngine( engine );
        engine.setService( catalina ); // needed to prevent NPE in ApplicationContext.populateSessionTrackingModes

        final Connector connector = catalina.createConnector( "localhost", port, false );
        catalina.addConnector( connector );

        return catalina;
    }

}


When  org.apache.catalina.connector.Connector.stopInternal is changed and sets the state to LifecycleState.MUST_DESTROY at the end this works as expected: the server socket is closed and free for further use.
Comment 1 Martin Grotzke 2010-11-28 20:20:10 UTC
Created attachment 26352 [details]
Add setState(LifecycleState.MUST_DESTROY) to Connector.stopInternal (based on 7.0.4)
Comment 2 Mark Thomas 2010-11-29 08:47:21 UTC
Connector components can be started and stopped multiple times and setting MUST_DESTROY will break that. Therefore, the patch can't be used in its current form.

The Connectors and some of their associated components have non-standard lifecycles and I suspect that a change to one of the associated components will be required to fix this.
Comment 3 Martin Grotzke 2010-11-30 16:24:43 UTC
I debugged this and found the following path of execution:

Connector.stopInternal():
-> protocolHandler.stop() (Http11Protocol, stop() implemented in AbstractHttp11Protocol)
   -> endpoint.stop() (JIoEndpoint)
      -> pause()
      -> unlockAccept()
      -> shutdownExecutor()

A problem here seems to be that JIoEndpoint.stop() does not close the serverSocket (only destroy is doing this).

Therefore, if the implementation of AbstractHttp11Protocol.stop() is changed to invoke
  endpoint.destroy();
instead of endpoint.stop() also fixes the issue.

Not sure if this is the correct solution.

How can we proceed with this?

Thanx && Cheers,
Martin
Comment 4 Mark Thomas 2010-11-30 17:52:07 UTC
I'm working on this at the moment (although work and infrastructure stuff may distract me for the next few days). I have already sorted out the MapperListener, the ProtocolHandler and Endpoints are next but they will take a little longer as all five of the connectors will probably need to be changed together.

From what I have looked at so far most of init() needs to move into start(), most of destroy() into stop() and any JMX stuff needs to move to init() / destroy().

The new Lifecycle code can't be used directly as it will create a dependency on Catalina from the Connectors but it should be possible to re-use most of the ideas and probably some of the code.
Comment 5 Martin Grotzke 2010-12-01 03:24:07 UTC
Ok, great you're on it already!
Comment 6 Mark Thomas 2010-12-01 13:38:53 UTC
It wasn't as much work as I feared. Pretty much all a simple copy and paste.

The fix has been applied to 7.0.x and will be included in 7.0.6 onwards.
Comment 7 Martin Grotzke 2010-12-02 03:11:32 UTC
Great, thanx a lot for fixing this!
Comment 8 Mark Thomas 2010-12-03 13:00:10 UTC
The original fix is going to have to be reverted. It caused issues with jsvc (bug 50406) since the privileged ports need to be bound before the user id switches to the non-root user.
Comment 9 Martin Grotzke 2010-12-08 19:15:46 UTC
D'oh. Is there another solution in sight?
Comment 10 Mark Thomas 2010-12-09 06:32:46 UTC
Yep. Working on it (well the clean up prior to implementing the fix) now. A bindOnInit param will be added to the connector that defaults to true. By default the connector will bind/unbind on init()/destroy(). Set it to false and the connector will bind/unbind on start)_/stop()

I could have hacked this in fairly quickly but the connector/protocol/endpoint code is long overdue a clean-up and this issue was the trigger for getting that done. Connector and protocol are cleaned up. Starting on the endpoints now.
Comment 11 Mark Thomas 2010-12-09 14:54:52 UTC
And fixed again. You want to set the bindOnInit parameter to false to get the behaviour you want.
Comment 12 Sylvain Laurent 2010-12-09 15:30:05 UTC
I think your changes broke /manager/status , I get the following exception with rev 1044114 :


SEVERE: Servlet.service() for servlet [Status] in context with path [/manager] threw exception [javax.management.AttributeNotFoundException:  Cannot find attribute maxTime for org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler@2377ab84] with root cause
javax.management.AttributeNotFoundException:  Cannot find attribute maxTime for org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler@2377ab84
	at org.apache.tomcat.util.modeler.ManagedBean.getGetter(ManagedBean.java:494)
	at org.apache.tomcat.util.modeler.BaseModelMBean.getAttribute(BaseModelMBean.java:180)
	at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getAttribute(DefaultMBeanServerInterceptor.java:666)
	at com.sun.jmx.mbeanserver.JmxMBeanServer.getAttribute(JmxMBeanServer.java:638)
	at org.apache.catalina.manager.StatusTransformer.writeConnectorState(StatusTransformer.java:275)
	at org.apache.catalina.manager.StatusManagerServlet.doGet(StatusManagerServlet.java:276)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:623)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:724)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:306)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:210)
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:240)
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:161)
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:561)
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:164)
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:108)
	at org.apache.catalina.valves.AccessLogValve.invoke(AccessLogValve.java:558)
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:118)
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:379)
	at org.apache.coyote.http11.Http11Processor.process(Http11Processor.java:243)
	at org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler.process(Http11Protocol.java:179)
	at org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler.process(Http11Protocol.java:157)
	at org.apache.tomcat.util.net.JIoEndpoint$SocketProcessor.run(JIoEndpoint.java:283)
	at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
	at java.lang.Thread.run(Thread.java:680)
Comment 13 Mark Thomas 2010-12-09 16:12:08 UTC
Re-factoring goof - fixed.
Comment 14 Martin Grotzke 2010-12-09 19:24:22 UTC
(In reply to comment #13)
> Re-factoring goof - fixed.

Great, thanx a lot for your work! Now I ran into another issue, will submit this separately (#50448).