tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Rossbach ...@objektpark.de>
Subject Re: memory leak with shrinking thread pools
Date Thu, 12 Apr 2007 13:18:27 GMT
Hi Filip,


I have test your patch with my comet testclient.  It seems not  
working.  The RequestProcessors JMX Objects aren't deregistered:

Access Log:

'127.0.0.1:30014' 2007-04-12 13:05:46 /cometchat/?null - - - 200  0.001
'127.0.0.1:30014' 2007-04-12 13:05:46 /cometchat/chat?null 'connect'  
'TestClient0' - 200 '9A46086F29768BA775FBD2771D8614BC.tomcat6' 0.001
'127.0.0.1:30014' 2007-04-12 13:05:46 /cometchat/chat?null 'send'  
'TestClient0' '0-1176383146864' 200  
'9A46086F29768BA775FBD2771D8614BC.tomcat6' 0.002
'127.0.0.1:30014' 2007-04-12 13:05:46 /cometchat/chat?null 'connect'  
'TestClient0' - 200 '9A46086F29768BA775FBD2771D8614BC.tomcat6' 0.000
'127.0.0.1:30014' 2007-04-12 13:05:51 /cometchat/chat?null  
'disconnect' 'TestClient0' - 200  
'9A46086F29768BA775FBD2771D8614BC.tomcat6' 0.001

Server.xml config:
...
     <Listener  
className="org.apache.catalina.core.AprLifecycleListener"  
SSLEngine="on" />
     <Listener className="org.apache.catalina.core.JasperListener" />
...
     <Service name="Catalina">
         <Connector port="30011"
                    URIEncoding="UTF-8"
		           useExecutor="false"
		           minSpareThreads="150"
		           maxSpareThreads="150"
		           maxThreads="150"
			       connectionTimeout="300000"
                     
protocol="org.apache.coyote.http11.Http11AprProtocol"/>
			       pollerSize="1024" />
         <Connector port="30014"
                    URIEncoding="UTF-8"
		           useExecutor="false"
		           minSpareThreads="150"
		           maxSpareThreads="150"
		           maxThreads="150"
			       connectionTimeout="300000"
                     
protocol="org.apache.coyote.http11.Http11NioProtocol"/>
...
</Server>


With NIO I see the following event log:

Event received connect BEGIN
Getting handler for action connect
New client connected TestClient0
Event received send BEGIN
Getting handler for action send
Sending message from TestClient0 message: 0-1176383115466
Disconnecting client (commit)
Sent 1 messages to connected client TestClient0
Event received connect BEGIN
Getting handler for action connect
Client reconnected: TestClient0
Event received disconnect BEGIN
Getting handler for action disconnect
Closing empty room 0
Disconnecting client (commit)
Remove client TestClient0. 0 clients remain.

After every test  three more JMX RequestProcessors are registered.
I don't see the Comet END events like the APR Connector send to my  
ChatServlet.

APR Connector Log

Event received connect BEGIN
Getting handler for action connect
New client connected TestClient0
Event received send BEGIN
Getting handler for action send
Sending message from TestClient0 message: 0-1176383857840
Disconnecting client (commit)
Sent 1 messages to connected client TestClient0
Event received connect END
Getting handler for action connect
Disconnecting client TestClient0
Event received connect BEGIN
Getting handler for action connect
Client reconnected: TestClient0
Event received disconnect BEGIN
Getting handler for action disconnect
Closing empty room 0
Disconnecting client (commit)
Remove client TestClient0. 0 clients remain.
Event received connect END
Getting handler for action connect

Regards
Peter


Am 12.04.2007 um 05:48 schrieb Filip Hanik - Dev Lists:

> Here is a revised patch for the memory leak.
> One thing I noticed is that it goes a little farther than I think  
> it does.
> Since every request processor gets registered with JMX, I just  
> couldn't find a spot where it was unregistered.
> And since the name was dynamic, ie based on the "count++" variable,  
> there would be no way to unregister unless you knew the name.
>
> http://people.apache.org/~fhanik/mem-leak-diff.patch
>
> I suspect, that this memory leak also exists with the old thread  
> pool, when you restart the endpoint in a running environment. At  
> that time, all the threads get recycled, and most stuff gets  
> unregistered, except the RequestInfo objects.
>
> In this patch, I modified the usage of the recycledProcessor cache,  
> to have a limit, and also to deregister objects should it be needed.
>
> Personally, I'm still really confused about how everything gets  
> registered with the MBean server and then holds hard references  
> into the code itself.
> On one note, I think the JMX stuff could simply have weak  
> references, as in my old patch, but that could have other  
> consequences.
>
> Please comment on this patch, I'm planning on committing it  
> tomorrow (Thursday) as it seems to work well for me.
>
> Filip
>
>
> Filip Hanik - Dev Lists wrote:
> Index: java/org/apache/coyote/http11/Http11NioProtocol.java
> ===================================================================
> --- java/org/apache/coyote/http11/Http11NioProtocol.java	(revision  
> 527526)
> +++ java/org/apache/coyote/http11/Http11NioProtocol.java	(working  
> copy)
> @@ -22,7 +22,9 @@
>  import java.util.Hashtable;
>  import java.util.Iterator;
>  import java.util.concurrent.ConcurrentHashMap;
> +import java.util.concurrent.ConcurrentLinkedQueue;
>  import java.util.concurrent.Executor;
> +import java.util.concurrent.atomic.AtomicInteger;
>  import javax.management.MBeanRegistration;
>  import javax.management.MBeanServer;
>  import javax.management.ObjectName;
> @@ -212,6 +214,7 @@
>      private int timeout = 300000;   // 5 minutes as in Apache  
> HTTPD server
>      private int maxSavePostSize = 4 * 1024;
>      private int maxHttpHeaderSize = 8 * 1024;
> +    protected int processorCache = 200; //max number of  
> Http11NioProcessor objects cached
>      private int socketCloseDelay=-1;
>      private boolean disableUploadTimeout = true;
>      private int socketBuffer = 9000;
> @@ -534,6 +537,10 @@
>          setAttribute("timeout", "" + timeouts);
>      }
>
> +    public void setProcessorCache(int processorCache) {
> +        this.processorCache = processorCache;
> +    }
> +
>      public void setOomParachute(int oomParachute) {
>          ep.setOomParachute(oomParachute);
>          setAttribute("oomParachute",oomParachute);
> @@ -584,8 +591,40 @@
>              new ThreadLocal<Http11NioProcessor>();
>          protected ConcurrentHashMap<NioChannel,  
> Http11NioProcessor> connections =
>              new ConcurrentHashMap<NioChannel, Http11NioProcessor>();
> -        protected java.util.Stack<Http11NioProcessor>  
> recycledProcessors =
> -            new java.util.Stack<Http11NioProcessor>();
> +        protected ConcurrentLinkedQueue<Http11NioProcessor>  
> recycledProcessors = new ConcurrentLinkedQueue<Http11NioProcessor>() {
> +            protected AtomicInteger size = new AtomicInteger(0);
> +            public boolean offer(Http11NioProcessor processor) {
> +                boolean offer = proto.processorCache==-1? 
> true:size.get() < proto.processorCache;
> +                //avoid over growing our cache or add after we  
> have stopped
> +                boolean result = false;
> +                if ( offer ) {
> +                    result = super.offer(processor);
> +                    if ( result ) {
> +                        size.incrementAndGet();
> +                    }
> +                }
> +                if (!result) deregister(processor);
> +                return result;
> +            }
> +
> +            public Http11NioProcessor poll() {
> +                Http11NioProcessor result = super.poll();
> +                if ( result != null ) {
> +                    size.decrementAndGet();
> +                }
> +                return result;
> +            }
> +
> +            public void clear() {
> +                Http11NioProcessor next = poll();
> +                while ( next != null ) {
> +                    deregister(next);
> +                    next = poll();
> +                }
> +                super.clear();
> +                size.set(0);
> +            }
> +        };
>
>          Http11ConnectionHandler(Http11NioProtocol proto) {
>              this.proto = proto;
> @@ -626,7 +665,7 @@
>                  } finally {
>                      if (state != SocketState.LONG) {
>                          connections.remove(socket);
> -                        recycledProcessors.push(result);
> +                        if (proto.ep.getUseExecutor())  
> recycledProcessors.offer(result);
>                      }
>                  }
>              }
> @@ -636,14 +675,10 @@
>          public SocketState process(NioChannel socket) {
>              Http11NioProcessor processor = null;
>              try {
> -                processor = (Http11NioProcessor) localProcessor.get 
> ();
> +                if ( !proto.ep.getUseExecutor() ) processor =  
> (Http11NioProcessor) localProcessor.get();
>                  if (processor == null) {
> -                    synchronized (recycledProcessors) {
> -                        if (!recycledProcessors.isEmpty()) {
> -                            processor = recycledProcessors.pop();
> -                            localProcessor.set(processor);
> -                        }
> -                    }
> +                    processor = recycledProcessors.poll();
> +                    if ( !proto.ep.getUseExecutor() )  
> localProcessor.set(processor);
>                  }
>                  if (processor == null) {
>                      processor = createProcessor();
> @@ -670,8 +705,10 @@
>                      // processed by this thread will use either a  
> new or a recycled
>                      // processor.
>                      connections.put(socket, processor);
> -                    localProcessor.set(null);
> +                    if ( !proto.ep.getUseExecutor() )  
> localProcessor.set(null);
>                      socket.getPoller().add(socket);
> +                } else if ( proto.ep.getUseExecutor() ) {
> +                    recycledProcessors.offer(processor);
>                  }
>                  return state;
>
> @@ -717,25 +754,49 @@
>              processor.setSocketBuffer(proto.socketBuffer);
>              processor.setMaxSavePostSize(proto.maxSavePostSize);
>              processor.setServer(proto.server);
> -            localProcessor.set(processor);
> +            if ( !proto.ep.getUseExecutor() ) localProcessor.set 
> (processor);
> +            register(processor);
> +            return processor;
> +        }
> +
> +        public void register(Http11NioProcessor processor) {
>              if (proto.getDomain() != null) {
>                  synchronized (this) {
>                      try {
>                          RequestInfo rp = processor.getRequest 
> ().getRequestProcessor();
>                          rp.setGlobalProcessor(global);
>                          ObjectName rpName = new ObjectName
> -                        (proto.getDomain() +  
> ":type=RequestProcessor,worker="
> -                                + proto.getName() +  
> ",name=HttpRequest" + count++);
> +                            (proto.getDomain() +  
> ":type=RequestProcessor,worker="
> +                             + proto.getName() +  
> ",name=HttpRequest" + count++);
>                          Registry.getRegistry(null,  
> null).registerComponent(rp, rpName, null);
> +                        rp.setRpName(rpName);
>                      } catch (Exception e) {
>                          log.warn("Error registering request");
>                      }
>                  }
>              }
> -            return processor;
>          }
> +
> +        public void deregister(Http11NioProcessor processor) {
> +            if (proto.getDomain() != null) {
> +                synchronized (this) {
> +                    try {
> +                        RequestInfo rp = processor.getRequest 
> ().getRequestProcessor();
> +                        rp.setGlobalProcessor(null);
> +                        ObjectName rpName = rp.getRpName();
> +                        Registry.getRegistry(null,  
> null).unregisterComponent(rpName);
> +                        rp.setRpName(null);
> +                    } catch (Exception e) {
> +                        log.warn("Error unregistering request", e);
> +                    }
> +                }
> +            }
> +        }
> +
>      }
> +
>
> +
>      protected static org.apache.juli.logging.Log log
>          = org.apache.juli.logging.LogFactory.getLog 
> (Http11NioProtocol.class);
>
> @@ -753,6 +814,10 @@
>          return domain;
>      }
>
> +    public int getProcessorCache() {
> +        return processorCache;
> +    }
> +
>      public int getOomParachute() {
>          return ep.getOomParachute();
>      }
> Index: java/org/apache/coyote/RequestInfo.java
> ===================================================================
> --- java/org/apache/coyote/RequestInfo.java	(revision 527526)
> +++ java/org/apache/coyote/RequestInfo.java	(working copy)
> @@ -17,7 +17,9 @@
>
>  package org.apache.coyote;
>
> +import javax.management.ObjectName;
>
> +
>  /**
>   * Structure holding the Request and Response objects. It also  
> holds statistical
>   * informations about request processing and provide management  
> informations
> @@ -63,6 +65,7 @@
>      Response res;
>      int stage = Constants.STAGE_NEW;
>      String workerThreadName;
> +    ObjectName rpName;
>
>      // -------------------- Information about the current request   
> -----------
>      // This is usefull for long-running requests only
> @@ -217,7 +220,15 @@
>          return workerThreadName;
>      }
>
> +    public ObjectName getRpName() {
> +        return rpName;
> +    }
> +
>      public void setWorkerThreadName(String workerThreadName) {
>          this.workerThreadName = workerThreadName;
>      }
> +
> +    public void setRpName(ObjectName rpName) {
> +        this.rpName = rpName;
> +    }
>  }
> Index: java/org/apache/tomcat/util/net/NioEndpoint.java
> ===================================================================
> --- java/org/apache/tomcat/util/net/NioEndpoint.java	(revision 527526)
> +++ java/org/apache/tomcat/util/net/NioEndpoint.java	(working copy)
> @@ -341,7 +341,7 @@
>
>      protected boolean useExecutor = true;
>      public void setUseExecutor(boolean useexec) { useExecutor =  
> useexec;}
> -    public boolean getUseExecutor() { return useExecutor;}
> +    public boolean getUseExecutor() { return useExecutor ||  
> (executor!=null);}
>
>      /**
>       * Maximum amount of worker threads.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message