Some time ago, I had to troubleshoot a nasty memory leak problem around connection pooling, where JVM heap space kept growing despite no incoming traffic. We were using the Apache Commons Pools library to manage the pool but we also extended to customize the validation of pooled object to ensure only pooled objects of less than certain age were kept. A ConcurrentHashMap instance was used to store the pooled object and its creation time, so when it was time to validate a pooled object, we would get the object from this hash map and check its age. A bug in this logic resulted in the hash map growing to heap space exhaustion. In particular, it was bug in equals method. In this blog, I summarize what I learnt:
Suppose:
You have an interface and a class that implements it;
public interface SomeInterface{}
public class ObjectP implements SomeInterface
{
public String str;
public ObjectP(String str)
{
this.str = str;
}
@Override
public String toString()
{
return str;
}
public String toUpperCase()
{
return str.toUpperCase();
}
@Override
public boolean equals(Object o)
{
System.out.println("ObjectP.equals invoked");
return str.equals(((ObjectP)o).str);
}
}
You have an TestInvocationHandler that acts a proxy to ObjectP:
public class TestInvocationHandler<T> implements InvocationHandler
{
T t = null;
public TestInvocationHandler(T t)
{
this.t = t;
}
@Override
public Object invoke(Object arg0, Method method, Object[] args)
throws Throwable {
if (method.getName().equals("equals")) {
System.out.println("equals method invoked. args[0].getClass()=" + args[0].getClass());
System.out.println("this object = " + this);
boolean result = this.equals(args[0]);
System.out.println("equals method returned " + result);
return result;
} else if (method.getName().equals("hashCode")) {
return this.hashCode();
} else if (method.getName().equals("toString")) {
return t.toString();
} else {
return invokeInternal(method, args);
}
}
private Object invokeInternal(Method method, Object[] args) throws Exception, IllegalArgumentException, InvocationTargetException
{
return method.invoke(t, args);
}
}
You create a Proxy class as follows:
TestInvocationHandler<ObjectP> h1 = new TestInvocationHandler<ObjectP>(new ObjectP("hello"));
TestInvocationHandler<ObjectP> h2 = new TestInvocationHandler<ObjectP>(new ObjectP("world"));
Proxy p1 = (Proxy) Proxy.newProxyInstance(ObjectP.class.getClassLoader(), new Class[] { SomeInterface.class }, h1);
What should p1.equals(p1) be? The answer is false. Here is the output of p1.equals(p1)
equals method invoked. args[0].getClass()=class com.sun.proxy.$Proxy0
this object = ProxyClassTest$TestInvocationHandler@1b16e52
equals method returned false
That is, this.equals(args[0])is equivalent of saying TestInvocationHandler.equals(Proxy) which always returns false.
This bug resulted in ConcurrentHashMap.remove(object) to do nothing which resulted in the hash map to grow till the heap space was full.
The fix was simple; args[0].equals(this) instead of this.equals(args[0]) so that we do RetryingInvocationHandler.equals(RetryingInvocationHandler):
if (method.getName().equals("equals")){
System.out.println("equals method invoked. args[0].getClass()=" + args[0].getClass());
System.out.println("this object = " + this);
boolean result = args[0].equals(this);
System.out.println("equals method returned " + result);
return result;
}
Suppose:
You have an interface and a class that implements it;
public interface SomeInterface{}
public class ObjectP implements SomeInterface
{
public String str;
public ObjectP(String str)
{
this.str = str;
}
@Override
public String toString()
{
return str;
}
public String toUpperCase()
{
return str.toUpperCase();
}
@Override
public boolean equals(Object o)
{
System.out.println("ObjectP.equals invoked");
return str.equals(((ObjectP)o).str);
}
}
You have an TestInvocationHandler that acts a proxy to ObjectP:
public class TestInvocationHandler<T> implements InvocationHandler
{
T t = null;
public TestInvocationHandler(T t)
{
this.t = t;
}
@Override
public Object invoke(Object arg0, Method method, Object[] args)
throws Throwable {
if (method.getName().equals("equals")) {
System.out.println("equals method invoked. args[0].getClass()=" + args[0].getClass());
System.out.println("this object = " + this);
boolean result = this.equals(args[0]);
System.out.println("equals method returned " + result);
return result;
} else if (method.getName().equals("hashCode")) {
return this.hashCode();
} else if (method.getName().equals("toString")) {
return t.toString();
} else {
return invokeInternal(method, args);
}
}
private Object invokeInternal(Method method, Object[] args) throws Exception, IllegalArgumentException, InvocationTargetException
{
return method.invoke(t, args);
}
}
You create a Proxy class as follows:
TestInvocationHandler<ObjectP> h1 = new TestInvocationHandler<ObjectP>(new ObjectP("hello"));
TestInvocationHandler<ObjectP> h2 = new TestInvocationHandler<ObjectP>(new ObjectP("world"));
Proxy p1 = (Proxy) Proxy.newProxyInstance(ObjectP.class.getClassLoader(), new Class[] { SomeInterface.class }, h1);
What should p1.equals(p1) be? The answer is false. Here is the output of p1.equals(p1)
equals method invoked. args[0].getClass()=class com.sun.proxy.$Proxy0
this object = ProxyClassTest$TestInvocationHandler@1b16e52
equals method returned false
That is, this.equals(args[0])is equivalent of saying TestInvocationHandler.equals(Proxy) which always returns false.
This bug resulted in ConcurrentHashMap.remove(object) to do nothing which resulted in the hash map to grow till the heap space was full.
The fix was simple; args[0].equals(this) instead of this.equals(args[0]) so that we do RetryingInvocationHandler.equals(RetryingInvocationHandler):
if (method.getName().equals("equals")){
System.out.println("equals method invoked. args[0].getClass()=" + args[0].getClass());
System.out.println("this object = " + this);
boolean result = args[0].equals(this);
System.out.println("equals method returned " + result);
return result;
}
No comments:
Post a Comment