[
https://issues.apache.org/jira/browse/POOL-315?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Buğra Gedik updated POOL-315:
-----------------------------
Description:
The {{close}} method is implemented as follows:
{code}
public void close() {
if(!this.isClosed()) {
Object var1 = this.closeLock;
synchronized(this.closeLock) {
if(!this.isClosed()) {
this.startEvictor(-1L);
this.closed = true;
this.clear();
this.jmxUnregister();
this.idleObjects.interuptTakeWaiters();
}
}
}
}
{code}
The line {{this.startEvictor(-1L);}} calls
{{EvictionTimer.cancel(this.evictor);}} from {{BaseGenericObjectPool}} and that
in turn calls the following method in {{EvictionTimer}}:
{code}
static synchronized void cancel(TimerTask task) {
task.cancel();
--_usageCount;
if(_usageCount == 0) {
_timer.cancel();
_timer = null;
}
}
{code}
Here, {{_timer}} is a {{java.util.TimerTask}}. If you look at the documentation
of it, you'll see that it does not block on the currently executing task. Even
though {{task.cancel()}} is called, there is no code to wait for it to
complete. The end result is that, even if you close the pool, there may still
be an eviction thread running. Yes, it will eventually go away, but in some
rare cases it takes a bit of time to go away.
When running code under Tomcat, this results in the
{{"commons-pool-EvictionTimer"}} thread (created in the class
{{EvictionTimer}}) to be reported as leaking, despite the pool being closed.
This happens rarely, since most of the time the timer thread goes away before
Tomcat checks for leaking threads.
In my opinion a fix can be put into {{startEvictor}} in
{{BaseGenericObjectPool}}:
{code}
final void startEvictor(long delay) {
synchronized (evictionLock) {
if (null != evictor) {
EvictionTimer.cancel(evictor);
// HERE: evictor.waitForCompletion();
evictor = null;
evictionIterator = null;
}
if (delay > 0) {
evictor = new Evictor();
EvictionTimer.schedule(evictor, delay, delay);
}
}
}
{code}
Here is an example that illustrates the problem:
{code:bar=false}
import static org.junit.Assert.assertFalse;
import org.apache.commons.pool2.PooledObject;
import org.apache.commons.pool2.PooledObjectFactory;
import org.apache.commons.pool2.impl.DefaultPooledObject;
import org.apache.commons.pool2.impl.GenericObjectPool;
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import org.junit.Test;
public class PoolTest
{
private static final CharSequence COMMONS_POOL_EVICTIONS_TIMER_THREAD_NAME =
"commons-pool-EvictionTimer";
private static final long EVICTION_PERIOD_IN_MILLIS = 100;
private static class Foo
{
}
private static class PooledFooFactory implements PooledObjectFactory<Foo>
{
private static final long VALIDATION_WAIT_IN_MILLIS = 1000;
@Override
public PooledObject<Foo> makeObject() throws Exception
{
return new DefaultPooledObject<>(new Foo());
}
@Override
public void destroyObject(
PooledObject<Foo> pooledObject) throws Exception
{
}
@Override
public boolean validateObject(
PooledObject<Foo> pooledObject)
{
try
{
Thread.sleep(VALIDATION_WAIT_IN_MILLIS);
}
catch (final InterruptedException e)
{
Thread.interrupted();
}
return false;
}
@Override
public void activateObject(
PooledObject<Foo> pooledObject) throws Exception
{
}
@Override
public void passivateObject(
PooledObject<Foo> pooledObject) throws Exception
{
}
}
@Test
public void testPool() throws Exception
{
final GenericObjectPoolConfig poolConfig =
new GenericObjectPoolConfig();
poolConfig.setTestWhileIdle(true /* testWhileIdle */);
final PooledFooFactory pooledFooFactory = new PooledFooFactory();
GenericObjectPool<Foo> pool = null;
try
{
pool = new GenericObjectPool<>(pooledFooFactory, poolConfig);
pool.setTimeBetweenEvictionRunsMillis(EVICTION_PERIOD_IN_MILLIS);
pool.addObject();
try
{
Thread.sleep(EVICTION_PERIOD_IN_MILLIS);
}
catch (final InterruptedException e)
{
Thread.interrupted();
}
}
finally
{
if (pool != null)
{
pool.close();
}
}
final Thread[] threads = new Thread[Thread.activeCount()];
Thread.enumerate(threads);
for (final Thread thread : threads)
{
if (thread == null)
{
continue;
}
assertFalse(thread.getName()
.contains(COMMONS_POOL_EVICTIONS_TIMER_THREAD_NAME));
}
}
}
{code}
was:
The {{close}} method is implemented as follows:
{code}
public void close() {
if(!this.isClosed()) {
Object var1 = this.closeLock;
synchronized(this.closeLock) {
if(!this.isClosed()) {
this.startEvictor(-1L);
this.closed = true;
this.clear();
this.jmxUnregister();
this.idleObjects.interuptTakeWaiters();
}
}
}
}
{code}
The line {{this.startEvictor(-1L);}} calls
{{EvictionTimer.cancel(this.evictor);}} from {{BaseGenericObjectPool}} and that
in turn calls the following method in {{EvictionTimer}}:
{code|bar=false}
static synchronized void cancel(TimerTask task) {
task.cancel();
--_usageCount;
if(_usageCount == 0) {
_timer.cancel();
_timer = null;
}
}
{code}
Here, {{_timer}} is a {{java.util.TimerTask}}. If you look at the documentation
of it, you'll see that it does not block on the currently executing task. Even
though {{task.cancel()}} is called, there is no code to wait for it to
complete. The end result is that, even if you close the pool, there may still
be an eviction thread running. Yes, it will eventually go away, but in some
rare cases it takes a bit of time to go away.
When running code under Tomcat, this results in the
{{"commons-pool-EvictionTimer"}} thread (created in the class
{{EvictionTimer}}) to be reported as leaking, despite the pool being closed.
This happens rarely, since most of the time the timer thread goes away before
Tomcat checks for leaking threads.
In my opinion a fix can be put into {{startEvictor}} in
{{BaseGenericObjectPool}}:
{code}
final void startEvictor(long delay) {
synchronized (evictionLock) {
if (null != evictor) {
EvictionTimer.cancel(evictor);
// HERE: evictor.waitForCompletion();
evictor = null;
evictionIterator = null;
}
if (delay > 0) {
evictor = new Evictor();
EvictionTimer.schedule(evictor, delay, delay);
}
}
}
{code}
Here is an example that illustrates the problem:
{code}
import static org.junit.Assert.assertFalse;
import org.apache.commons.pool2.PooledObject;
import org.apache.commons.pool2.PooledObjectFactory;
import org.apache.commons.pool2.impl.DefaultPooledObject;
import org.apache.commons.pool2.impl.GenericObjectPool;
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import org.junit.Test;
public class PoolTest
{
private static final CharSequence COMMONS_POOL_EVICTIONS_TIMER_THREAD_NAME =
"commons-pool-EvictionTimer";
private static final long EVICTION_PERIOD_IN_MILLIS = 100;
private static class Foo
{
}
private static class PooledFooFactory implements PooledObjectFactory<Foo>
{
private static final long VALIDATION_WAIT_IN_MILLIS = 1000;
@Override
public PooledObject<Foo> makeObject() throws Exception
{
return new DefaultPooledObject<>(new Foo());
}
@Override
public void destroyObject(
PooledObject<Foo> pooledObject) throws Exception
{
}
@Override
public boolean validateObject(
PooledObject<Foo> pooledObject)
{
try
{
Thread.sleep(VALIDATION_WAIT_IN_MILLIS);
}
catch (final InterruptedException e)
{
Thread.interrupted();
}
return false;
}
@Override
public void activateObject(
PooledObject<Foo> pooledObject) throws Exception
{
}
@Override
public void passivateObject(
PooledObject<Foo> pooledObject) throws Exception
{
}
}
@Test
public void testPool() throws Exception
{
final GenericObjectPoolConfig poolConfig =
new GenericObjectPoolConfig();
poolConfig.setTestWhileIdle(true /* testWhileIdle */);
final PooledFooFactory pooledFooFactory = new PooledFooFactory();
GenericObjectPool<Foo> pool = null;
try
{
pool = new GenericObjectPool<>(pooledFooFactory, poolConfig);
pool.setTimeBetweenEvictionRunsMillis(EVICTION_PERIOD_IN_MILLIS);
pool.addObject();
try
{
Thread.sleep(EVICTION_PERIOD_IN_MILLIS);
}
catch (final InterruptedException e)
{
Thread.interrupted();
}
}
finally
{
if (pool != null)
{
pool.close();
}
}
final Thread[] threads = new Thread[Thread.activeCount()];
Thread.enumerate(threads);
for (final Thread thread : threads)
{
if (thread == null)
{
continue;
}
assertFalse(thread.getName()
.contains(COMMONS_POOL_EVICTIONS_TIMER_THREAD_NAME));
}
}
}
{code}
> GenericObjectPool close() does not wait for the current eviction task
> ---------------------------------------------------------------------
>
> Key: POOL-315
> URL: https://issues.apache.org/jira/browse/POOL-315
> Project: Commons Pool
> Issue Type: Bug
> Reporter: Buğra Gedik
>
> The {{close}} method is implemented as follows:
> {code}
> public void close() {
> if(!this.isClosed()) {
> Object var1 = this.closeLock;
> synchronized(this.closeLock) {
> if(!this.isClosed()) {
> this.startEvictor(-1L);
> this.closed = true;
> this.clear();
> this.jmxUnregister();
> this.idleObjects.interuptTakeWaiters();
> }
> }
> }
> }
> {code}
> The line {{this.startEvictor(-1L);}} calls
> {{EvictionTimer.cancel(this.evictor);}} from {{BaseGenericObjectPool}} and
> that in turn calls the following method in {{EvictionTimer}}:
> {code}
> static synchronized void cancel(TimerTask task) {
> task.cancel();
> --_usageCount;
> if(_usageCount == 0) {
> _timer.cancel();
> _timer = null;
> }
> }
> {code}
> Here, {{_timer}} is a {{java.util.TimerTask}}. If you look at the
> documentation of it, you'll see that it does not block on the currently
> executing task. Even though {{task.cancel()}} is called, there is no code to
> wait for it to complete. The end result is that, even if you close the pool,
> there may still be an eviction thread running. Yes, it will eventually go
> away, but in some rare cases it takes a bit of time to go away.
> When running code under Tomcat, this results in the
> {{"commons-pool-EvictionTimer"}} thread (created in the class
> {{EvictionTimer}}) to be reported as leaking, despite the pool being closed.
> This happens rarely, since most of the time the timer thread goes away before
> Tomcat checks for leaking threads.
> In my opinion a fix can be put into {{startEvictor}} in
> {{BaseGenericObjectPool}}:
> {code}
> final void startEvictor(long delay) {
> synchronized (evictionLock) {
> if (null != evictor) {
> EvictionTimer.cancel(evictor);
> // HERE: evictor.waitForCompletion();
> evictor = null;
> evictionIterator = null;
> }
> if (delay > 0) {
> evictor = new Evictor();
> EvictionTimer.schedule(evictor, delay, delay);
> }
> }
> }
> {code}
> Here is an example that illustrates the problem:
> {code:bar=false}
> import static org.junit.Assert.assertFalse;
> import org.apache.commons.pool2.PooledObject;
> import org.apache.commons.pool2.PooledObjectFactory;
> import org.apache.commons.pool2.impl.DefaultPooledObject;
> import org.apache.commons.pool2.impl.GenericObjectPool;
> import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
> import org.junit.Test;
> public class PoolTest
> {
> private static final CharSequence
> COMMONS_POOL_EVICTIONS_TIMER_THREAD_NAME =
> "commons-pool-EvictionTimer";
> private static final long EVICTION_PERIOD_IN_MILLIS = 100;
> private static class Foo
> {
> }
> private static class PooledFooFactory implements PooledObjectFactory<Foo>
> {
> private static final long VALIDATION_WAIT_IN_MILLIS = 1000;
> @Override
> public PooledObject<Foo> makeObject() throws Exception
> {
> return new DefaultPooledObject<>(new Foo());
> }
> @Override
> public void destroyObject(
> PooledObject<Foo> pooledObject) throws Exception
> {
> }
> @Override
> public boolean validateObject(
> PooledObject<Foo> pooledObject)
> {
> try
> {
> Thread.sleep(VALIDATION_WAIT_IN_MILLIS);
> }
> catch (final InterruptedException e)
> {
> Thread.interrupted();
> }
> return false;
> }
> @Override
> public void activateObject(
> PooledObject<Foo> pooledObject) throws Exception
> {
> }
> @Override
> public void passivateObject(
> PooledObject<Foo> pooledObject) throws Exception
> {
> }
> }
> @Test
> public void testPool() throws Exception
> {
> final GenericObjectPoolConfig poolConfig =
> new GenericObjectPoolConfig();
> poolConfig.setTestWhileIdle(true /* testWhileIdle */);
> final PooledFooFactory pooledFooFactory = new PooledFooFactory();
> GenericObjectPool<Foo> pool = null;
> try
> {
> pool = new GenericObjectPool<>(pooledFooFactory, poolConfig);
> pool.setTimeBetweenEvictionRunsMillis(EVICTION_PERIOD_IN_MILLIS);
> pool.addObject();
> try
> {
> Thread.sleep(EVICTION_PERIOD_IN_MILLIS);
> }
> catch (final InterruptedException e)
> {
> Thread.interrupted();
> }
> }
> finally
> {
> if (pool != null)
> {
> pool.close();
> }
> }
> final Thread[] threads = new Thread[Thread.activeCount()];
> Thread.enumerate(threads);
> for (final Thread thread : threads)
> {
> if (thread == null)
> {
> continue;
> }
> assertFalse(thread.getName()
> .contains(COMMONS_POOL_EVICTIONS_TIMER_THREAD_NAME));
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
