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<WeakReference<RequestInfo>> processors=new
ArrayList<WeakReference<RequestInfo>>();
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<RequestInfo> reference = new
WeakReference<RequestInfo>(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<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- if( maxTime < rp.getMaxTime() ) maxTime=rp.getMaxTime();
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if( rp!=null && maxTime < rp.getMaxTime() )
maxTime=rp.getMaxTime();
}
return maxTime;
}
@@ -63,16 +67,16 @@
public synchronized void setMaxTime(long maxTime) {
deadMaxTime = maxTime;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- rp.setMaxTime(maxTime);
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null ) rp.setMaxTime(maxTime);
}
}
public synchronized long getProcessingTime() {
long time=deadProcessingTime;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- time += rp.getProcessingTime();
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null )time += rp.getProcessingTime();
}
return time;
}
@@ -80,16 +84,16 @@
public synchronized void setProcessingTime(long totalTime) {
deadProcessingTime = totalTime;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- rp.setProcessingTime( totalTime );
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null )rp.setProcessingTime( totalTime );
}
}
public synchronized int getRequestCount() {
int requestCount=deadRequestCount;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- requestCount += rp.getRequestCount();
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null )requestCount += rp.getRequestCount();
}
return requestCount;
}
@@ -97,16 +101,16 @@
public synchronized void setRequestCount(int requestCount) {
deadRequestCount = requestCount;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- rp.setRequestCount( requestCount );
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null )rp.setRequestCount( requestCount );
}
}
public synchronized int getErrorCount() {
int requestCount=deadErrorCount;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- requestCount += rp.getErrorCount();
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null )requestCount += rp.getErrorCount();
}
return requestCount;
}
@@ -114,16 +118,16 @@
public synchronized void setErrorCount(int errorCount) {
deadErrorCount = errorCount;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- rp.setErrorCount( errorCount);
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null )rp.setErrorCount( errorCount);
}
}
public synchronized long getBytesReceived() {
long bytes=deadBytesReceived;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- bytes += rp.getBytesReceived();
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null )bytes += rp.getBytesReceived();
}
return bytes;
}
@@ -131,16 +135,16 @@
public synchronized void setBytesReceived(long bytesReceived) {
deadBytesReceived = bytesReceived;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- rp.setBytesReceived( bytesReceived );
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null )rp.setBytesReceived( bytesReceived );
}
}
public synchronized long getBytesSent() {
long bytes=deadBytesSent;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- bytes += rp.getBytesSent();
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null )bytes += rp.getBytesSent();
}
return bytes;
}
@@ -148,8 +152,8 @@
public synchronized void setBytesSent(long bytesSent) {
deadBytesSent = bytesSent;
for( int i=0; i<processors.size(); i++ ) {
- RequestInfo rp=(RequestInfo)processors.get( i );
- rp.setBytesSent( bytesSent );
+ RequestInfo rp=(RequestInfo)processors.get( i ).get();
+ if ( rp!=null )rp.setBytesSent( bytesSent );
}
}
Index: java/org/apache/coyote/Request.java
===================================================================
--- java/org/apache/coyote/Request.java (revision 527526)
+++ java/org/apache/coyote/Request.java (working copy)
@@ -28,6 +28,7 @@
import org.apache.tomcat.util.http.Parameters;
import org.apache.tomcat.util.http.ContentType;
import org.apache.tomcat.util.http.Cookies;
+import java.lang.ref.WeakReference;
/**
* This is a low-level, efficient representation of a server request. Most
@@ -138,7 +139,7 @@
private HashMap attributes=new HashMap();
private Response response;
- private ActionHook hook;
+ private WeakReference<ActionHook> 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<ActionHook>(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<ActionHook> 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<ActionHook>(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]
