Re: memory leak with shrinking thread pools

2007-04-12 Thread Filip Hanik - Dev Lists

Peter Rossbach wrote:

HI Filip,

with your last checkin my trace changed but leak is there. Also I got 
a exception too:

yes, still working on it,
Filip


12.04.2007 17:20:30 org.apache.tomcat.util.net.NioSelectorPool 
getSharedSelector

INFO: Using a shared selector for servlet write/read
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-1176391230388
Disconnecting client (commit)
Sent 1 messages to connected client TestClient0
Event received connect BEGIN
Getting handler for action connect
Client reconnected: TestClient0
Event received connect END
Getting handler for action connect
Disconnecting client TestClient0
Disconnecting client (commit)
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 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-1176391294275
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.
Exception in thread "http-30014-2" java.lang.NullPointerException
at 
org.apache.tomcat.util.net.NioEndpoint$Worker.run(NioEndpoint.java:1863)

at java.lang.Thread.run(Thread.java:613)
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-1176391394010
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.
Exception in thread "http-30014-1" java.lang.NullPointerException
at 
org.apache.tomcat.util.net.NioEndpoint$Worker.run(NioEndpoint.java:1863)

at java.lang.Thread.run(Thread.java:613)


Regards
Peter



Am 12.04.2007 um 16:29 schrieb Filip Hanik - Dev Lists:


Peter Rossbach wrote:

Hi Filip,


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

So far the patch is only for NIO, I will make it for APR and JIO today.


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




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 commen

Re: memory leak with shrinking thread pools

2007-04-12 Thread Peter Rossbach

HI Filip,

with your last checkin my trace changed but leak is there. Also I got  
a exception too:


12.04.2007 17:20:30 org.apache.tomcat.util.net.NioSelectorPool  
getSharedSelector

INFO: Using a shared selector for servlet write/read
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-1176391230388
Disconnecting client (commit)
Sent 1 messages to connected client TestClient0
Event received connect BEGIN
Getting handler for action connect
Client reconnected: TestClient0
Event received connect END
Getting handler for action connect
Disconnecting client TestClient0
Disconnecting client (commit)
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 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-1176391294275
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.
Exception in thread "http-30014-2" java.lang.NullPointerException
at org.apache.tomcat.util.net.NioEndpoint$Worker.run 
(NioEndpoint.java:1863)

at java.lang.Thread.run(Thread.java:613)
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-1176391394010
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.
Exception in thread "http-30014-1" java.lang.NullPointerException
at org.apache.tomcat.util.net.NioEndpoint$Worker.run 
(NioEndpoint.java:1863)

at java.lang.Thread.run(Thread.java:613)


Regards
Peter



Am 12.04.2007 um 16:29 schrieb Filip Hanik - Dev Lists:


Peter Rossbach wrote:

Hi Filip,


I have test your patch with my comet testclient.  It seems not  
working.  The RequestProcessors JMX Objects aren't deregistered:
So far the patch is only for NIO, I will make it for APR and JIO  
today.


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




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 pl

Re: memory leak with shrinking thread pools

2007-04-12 Thread Peter Rossbach

Hi Filip,

I have testet with NIO and the memory leak is still there.

Peter



Am 12.04.2007 um 16:29 schrieb Filip Hanik - Dev Lists:


Peter Rossbach wrote:

Hi Filip,


I have test your patch with my comet testclient.  It seems not  
working.  The RequestProcessors JMX Objects aren't deregistered:
So far the patch is only for NIO, I will make it for APR and JIO  
today.


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:
...
className="org.apache.catalina.core.AprLifecycleListener"  
SSLEngine="on" />


...


protocol="org.apache.coyote.http11.Http11AprProtocol"/>

   pollerSize="1024" />

protocol="org.apache.coyote.http11.Http11NioProtocol"/>

...



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.

What steps do you take to see an "END" event?
Filip


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



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]






Re: memory leak with shrinking thread pools

2007-04-12 Thread Filip Hanik - Dev Lists

Peter Rossbach wrote:

Hi Filip,


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

So far the patch is only for NIO, I will make it for APR and JIO today.


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:
...
className="org.apache.catalina.core.AprLifecycleListener" 
SSLEngine="on" />


...

   
protocol="org.apache.coyote.http11.Http11AprProtocol"/>

   pollerSize="1024" />
   
protocol="org.apache.coyote.http11.Http11NioProtocol"/>

...



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.

What steps do you take to see an "END" event?
Filip


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



-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: memory leak with shrinking thread pools

2007-04-12 Thread Peter Rossbach

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:
...
className="org.apache.catalina.core.AprLifecycleListener"  
SSLEngine="on" />


...


protocol="org.apache.coyote.http11.Http11AprProtocol"/>

   pollerSize="1024" />

protocol="org.apache.coyote.http11.Http11NioProtocol"/>

...



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 = 30;   // 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 = 

Re: memory leak with shrinking thread pools

2007-04-12 Thread Remy Maucherat

Filip Hanik - Dev Lists wrote:

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.


It needs to be unregistered when (= if) it is discarded. However, the 
monitoring needs were there when the processor was associated with a 
thread. Now with comet and executors, this is not the case. So it may be 
a good idea to think and see if a similar monitoring capability with a 
different implementation could be provided in the future (otherwise, it 
should probably go away).



http://people.apache.org/~fhanik/mem-leak-diff.patch


In the immediate future, I think it's the best solution.

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.


It should have a mechanical limit already: it's the maximum amount of 
concurrent connections + the maximum amount of comet connections. Giving 
it a lower limit would mean creating processors, which is expensive. I 
suggest adding a parameter.


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.


IMO, weak references are too weird, plus it would not solve any 
registration issue.


Please comment on this patch, I'm planning on committing it tomorrow 
(Thursday) as it seems to work well for me.


Rémy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: memory leak with shrinking thread pools

2007-04-11 Thread 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 = 30;   // 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();
 protected ConcurrentHashMap 
connections =
 new ConcurrentHashMap();
-protected java.util.Stack recycledProcessors = 
-new java.util.Stack();
+protected ConcurrentLinkedQueue recycledProcessors 
= new ConcurrentLinkedQueue() {
+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 = 
(Http1

Re: memory leak with shrinking thread pools

2007-04-11 Thread Filip Hanik - Dev Lists

Remy Maucherat wrote:

Filip Hanik - Dev Lists wrote:
you're of course right, I think I simply need to get rid of the 
thread local cache, and use the processor cache instead.
do you have any idea of the performance penalty? I'd probably use a 
ConcurrentLinkedQueue or something instead of 
synchronized{processorCache}


There's a penalty already for using an executor (unfortunately). So 
there could be a check: if using an executor, then use a processor 
pool of some sort (a dumb synced stack could do it, but I don't know 
if it would be better or worse speed wise - maybe about the same since 
there's no read at all, only modifications of the "collection"), 
otherwise use the thread local. Maybe there could be a better 
solution, but it's probably the easy one.


I didn't think about this leak issue when I thought about supporting 
executors.
me neither, but then again, the thread local is not the real problem. 
its the requestinfo and requestgroupinfo objects, has the funky circular 
dependency, and ties it back all the way to the "handler". Had it not 
been for those, then there would have been no leak, since the objects 
would have been collected when the thread died, and the thread local 
reference disappeared.


Filip


Rémy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]






-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: memory leak with shrinking thread pools

2007-04-11 Thread Remy Maucherat

Filip Hanik - Dev Lists wrote:
you're of course right, I think I simply need to get rid of the thread 
local cache, and use the processor cache instead.
do you have any idea of the performance penalty? I'd probably use a 
ConcurrentLinkedQueue or something instead of synchronized{processorCache}


There's a penalty already for using an executor (unfortunately). So 
there could be a check: if using an executor, then use a processor pool 
of some sort (a dumb synced stack could do it, but I don't know if it 
would be better or worse speed wise - maybe about the same since there's 
no read at all, only modifications of the "collection"), otherwise use 
the thread local. Maybe there could be a better solution, but it's 
probably the easy one.


I didn't think about this leak issue when I thought about supporting 
executors.


Rémy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: memory leak with shrinking thread pools

2007-04-11 Thread Filip Hanik - Dev Lists

Remy Maucherat wrote:

Filip Hanik - Dev Lists wrote:
attached is an example patch using weak references that would solve 
this problem,

I'd like to get thoughts on this patch, please comment

if the attachment doesn't make it
http://people.apache.org/~fhanik/mem-leak-diff.patch


For sure it's not the right way to do it (the right way being 
unregistering the appropriate management artifacts when the associated 
resources are released - if it's not possible, then it means the 
design of the said resources is broken).
you're of course right, I think I simply need to get rid of the thread 
local cache, and use the processor cache instead.
do you have any idea of the performance penalty? I'd probably use a 
ConcurrentLinkedQueue or something instead of synchronized{processorCache}


Filip


Rémy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]






-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: memory leak with shrinking thread pools

2007-04-11 Thread Remy Maucherat

Filip Hanik - Dev Lists wrote:
attached is an example patch using weak references that would solve this 
problem,

I'd like to get thoughts on this patch, please comment

if the attachment doesn't make it
http://people.apache.org/~fhanik/mem-leak-diff.patch


For sure it's not the right way to do it (the right way being 
unregistering the appropriate management artifacts when the associated 
resources are released - if it's not possible, then it means the design 
of the said resources is broken).


Rémy

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: memory leak with shrinking thread pools

2007-04-11 Thread Filip Hanik - Dev Lists
attached is an example patch using weak references that would solve this 
problem,

I'd like to get thoughts on this patch, please comment

if the attachment doesn't make it
http://people.apache.org/~fhanik/mem-leak-diff.patch

Filip

Filip Hanik - Dev Lists wrote:

Thanks to Peter Rossbach for alerting me to this.

If we are using the Executor, and are using shrinking thread pools, 
the RequestGroupInfo collects these objects and never releases them.
The only reason we don't see the problem with the default thread pool, 
is cause it never shrinks, hence it never loses it's thread local 
reference to the object.


Current workarounds are:
1. maxThreads==minSpareThreads
2. Disable JMX

Example of reference tree
http://people.apache.org/~fhanik/shrinking-pool-leak.html

Filip

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]





Index: java/org/apache/coyote/RequestGroupInfo.java
===
--- java/org/apache/coyote/RequestGroupInfo.java(revision 527526)
+++ java/org/apache/coyote/RequestGroupInfo.java(working copy)
@@ -18,13 +18,14 @@
 package org.apache.coyote;
 
 import java.util.ArrayList;
+import java.lang.ref.WeakReference;
 
 /** This can be moved to top level ( eventually with a better name ).
  *  It is currently used only as a JMX artifact, to agregate the data
  *  collected from each RequestProcessor thread.
  */
 public class RequestGroupInfo {
-ArrayList processors=new ArrayList();
+ArrayList> processors=new 
ArrayList>();
 private long deadMaxTime = 0;
 private long deadProcessingTime = 0;
 private int deadRequestCount = 0;
@@ -33,7 +34,8 @@
 private long deadBytesSent = 0;
 
 public synchronized void addRequestProcessor( RequestInfo rp ) {
-processors.add( rp );
+WeakReference reference = new 
WeakReference(rp);
+processors.add( reference );
 }
 
 public synchronized void removeRequestProcessor( RequestInfo rp ) {
@@ -45,16 +47,18 @@
 deadErrorCount += rp.getErrorCount();
 deadBytesReceived += rp.getBytesReceived();
 deadBytesSent += rp.getBytesSent();
-
-processors.remove( rp );
+for (int i=processors.size()-1; i>=0; i-- ) {
+if ( processors.get(i).get()== null || 
processors.get(i).get().equals(rp) )
+processors.remove(rp);
+}
 }
 }
 
 public synchronized long getMaxTime() {
 long maxTime=deadMaxTime;
 for( int i=0; i hook;
 
 private int bytesRead=0;
 // Time of the request - usefull to avoid repeated calls to 
System.currentTime
@@ -341,14 +342,14 @@
 }
 
 public void action(ActionCode actionCode, Object param) {
-if( hook==null && response!=null )
-hook=response.getHook();
+if( (hook==null || hook.get()==null ) && response!=null )
+hook= new WeakReference(response.getHook());
 
 if (hook != null) {
 if( param==null ) 
-hook.action(actionCode, this);
+hook.get().action(actionCode, this);
 else
-hook.action(actionCode, param);
+hook.get().action(actionCode, param);
 }
 }
 
Index: java/org/apache/coyote/Response.java
===
--- java/org/apache/coyote/Response.java(revision 527526)
+++ java/org/apache/coyote/Response.java(working copy)
@@ -22,6 +22,7 @@
 
 import org.apache.tomcat.util.buf.ByteChunk;
 import org.apache.tomcat.util.http.MimeHeaders;
+import java.lang.ref.WeakReference;
 
 /**
  * Response object.
@@ -92,7 +93,7 @@
 /**
  * Action hook.
  */
-public ActionHook hook;
+public WeakReference hook;
 
 
 /**
@@ -150,12 +151,12 @@
 
 
 public ActionHook getHook() {
-return hook;
+return hook.get();
 }
 
 
 public void setHook(ActionHook hook) {
-this.hook = hook;
+this.hook = new WeakReference(hook);
 }
 
 
@@ -176,11 +177,12 @@
 
 
 public void action(ActionCode actionCode, Object param) {
-if (hook != null) {
+if (hook != null && hook.get()!=null) {
 if( param==null ) 
-hook.action(actionCode, this);
-else
-hook.action(actionCode, param);
+hook.get().action(actionCode, this);
+else {
+hook.get().action(actionCode, param);
+}
 }
 }
 

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

memory leak with shrinking thread pools

2007-04-11 Thread Filip Hanik - Dev Lists

Thanks to Peter Rossbach for alerting me to this.

If we are using the Executor, and are using shrinking thread pools, the 
RequestGroupInfo collects these objects and never releases them.
The only reason we don't see the problem with the default thread pool, 
is cause it never shrinks, hence it never loses it's thread local 
reference to the object.


Current workarounds are:
1. maxThreads==minSpareThreads
2. Disable JMX

Example of reference tree
http://people.apache.org/~fhanik/shrinking-pool-leak.html

Filip

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]