[
https://issues.apache.org/jira/browse/POOL-231?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13508841#comment-13508841
]
Phil Steitz commented on POOL-231:
----------------------------------
Sorry the original test that I included in the description (now edited) was
incorrect. Combining borrow / return loops caused duplicates in the obj array.
I am pretty sure now this is a bug and the description of how it happens is
correct. The test will pass if I patch invalidateObject in GOP to guard
destroy(p) using p's monitor and include a second allObjects check:
{code}
synchronized (p) {
if (allObjects.get(obj) != null) {
destroy(p);
}
}
{code}
That is probably not the best way to address the problem (probably better to
somehow guard using PooledObject state); but I think it does show that there is
a problem and where it is.
> GOP, GKOP invalidateObject is not threadsafe
> --------------------------------------------
>
> Key: POOL-231
> URL: https://issues.apache.org/jira/browse/POOL-231
> Project: Commons Pool
> Issue Type: Bug
> Affects Versions: Nightly Builds
> Reporter: Phil Steitz
> Fix For: 2.0
>
>
> There does not appear to be sufficient sync protection for the destroyCount
> and createCount counters when destroy is activated by invalidateObject in
> GOP, GKOP, v2 trunk. The test below fails when added to GOP tests. The
> destroyCount is over-incremented due (I think) to the fact that multiple
> threads can enter destroy before the object being invalidated is removed from
> allObjects. This problem was originally reported by Thomas Neidhart in a
> comment on POOL-213.
> {code}
> @Test
> public void testConcurrentInvalidate() throws Exception {
> // Get allObjects and idleObjects loaded with some instances
> final int nObjects = 1000;
> pool.setMaxTotal(nObjects);
> pool.setMaxIdle(nObjects);
> final Object[] obj = new Object[nObjects];
> for (int i = 0; i < nObjects; i++) {
> obj[i] = pool.borrowObject();
> }
> for (int i = 0; i < nObjects; i++) {
> if (i % 2 == 0) {
> pool.returnObject(obj[i]);
> }
> }
> final int nThreads = 100;
> final int nIterations = 100;
> final InvalidateThread[] threads = new InvalidateThread[nThreads];
> // Randomly generated list of distinct invalidation targets
> final ArrayList<Integer> targets = new ArrayList<Integer>();
> final Random random = new Random();
> for (int j = 0; j < nIterations; j++) {
> // Get a new invalidation target
> Integer targ = new Integer(random.nextInt(nObjects));
> while (targets.contains(targ)) {
> targ = new Integer(random.nextInt(nObjects));
> }
> targets.add(targ);
> // Launch nThreads threads all trying to invalidate the target
> for (int i = 0; i < nThreads; i++) {
> threads[i] = new InvalidateThread(pool, obj[targ]);
> }
> for (int i = 0; i < nThreads; i++) {
> new Thread(threads[i]).start();
> }
> boolean done = false;
> while (!done) {
> done = true;
> for (int i = 0; i < nThreads; i++) {
> done = done && threads[i].complete();
> }
> Thread.sleep(100);
> }
> }
> Assert.assertEquals(nIterations, pool.getDestroyedCount());
> }
>
> static class InvalidateThread implements Runnable {
> final private Object obj;
> final private ObjectPool<Object> pool;
> private boolean done = false;
> public InvalidateThread(ObjectPool<Object> pool, Object obj) {
> this.obj = obj;
> this.pool = pool;
> }
> public void run() {
> try {
> pool.invalidateObject(obj);
> } catch (IllegalStateException ex) {
> // Ignore
> } catch (Exception ex) {
> Assert.fail("Unexpected exception " + ex.toString());
> } finally {
> done = true;
> }
> }
> public boolean complete() {
> return done;
> }
> }
> {code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
