This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-pool.git
The following commit(s) were added to refs/heads/master by this push:
new 97a43787 [POOL-407] Threads get stuck when idleObjects list is empty
new 4473e617 Merge branch 'master' of
https://gitbox.apache.org/repos/asf/commons-pool.git
97a43787 is described below
commit 97a4378754845f0659f1f4e635af73a692d2bc9c
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Sun Oct 9 16:56:13 2022 -0400
[POOL-407] Threads get stuck when idleObjects list is empty
Refactor, expand, and add disabled test methods
---
.../apache/commons/pool2/pool407/KeyedPool407.java | 6 +++--
...Factory.java => KeyedPool407NormalFactory.java} | 4 ++--
...ory.java => KeyedPool407NullObjectFactory.java} | 23 +++++++-----------
... => KeyedPool407NullPoolableObjectFactory.java} | 24 +++++++------------
.../commons/pool2/pool407/KeyedPool407Test.java | 27 +++++++++++++++++-----
.../org/apache/commons/pool2/pool407/Pool407.java | 7 +++++-
...ol407Factory.java => Pool407NormalFactory.java} | 4 ++--
...7Factory.java => Pool407NullObjectFactory.java} | 23 +++++++-----------
....java => Pool407NullPoolableObjectFactory.java} | 25 ++++++++++++++++----
.../apache/commons/pool2/pool407/Pool407Test.java | 26 ++++++++++++++++-----
10 files changed, 100 insertions(+), 69 deletions(-)
diff --git a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407.java
b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407.java
index 1a65b0b7..c843e43e 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407.java
+++ b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407.java
@@ -17,11 +17,13 @@
package org.apache.commons.pool2.pool407;
+import org.apache.commons.pool2.BaseKeyedPooledObjectFactory;
import org.apache.commons.pool2.impl.GenericKeyedObjectPool;
public final class KeyedPool407 extends GenericKeyedObjectPool<String,
KeyedPool407Fixture, RuntimeException> {
- public KeyedPool407(final KeyedPool407Fixture fixture) {
- super(new KeyedPool407Factory(fixture), new KeyedPool407Config());
+ public KeyedPool407(final BaseKeyedPooledObjectFactory<String,
KeyedPool407Fixture, RuntimeException> factory) {
+ super(factory, new KeyedPool407Config());
}
+
}
diff --git
a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407NormalFactory.java
similarity index 91%
copy from
src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
copy to
src/test/java/org/apache/commons/pool2/pool407/KeyedPool407NormalFactory.java
index 950bf48a..41bd65c4 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
+++
b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407NormalFactory.java
@@ -21,11 +21,11 @@ import
org.apache.commons.pool2.BaseKeyedPooledObjectFactory;
import org.apache.commons.pool2.PooledObject;
import org.apache.commons.pool2.impl.DefaultPooledObject;
-public final class KeyedPool407Factory extends
BaseKeyedPooledObjectFactory<String, KeyedPool407Fixture, RuntimeException> {
+public final class KeyedPool407NormalFactory extends
BaseKeyedPooledObjectFactory<String, KeyedPool407Fixture, RuntimeException> {
private final KeyedPool407Fixture fixture;
- KeyedPool407Factory(final KeyedPool407Fixture fixture) {
+ KeyedPool407NormalFactory(final KeyedPool407Fixture fixture) {
this.fixture = fixture;
}
diff --git
a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407NullObjectFactory.java
similarity index 57%
copy from
src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
copy to
src/test/java/org/apache/commons/pool2/pool407/KeyedPool407NullObjectFactory.java
index 950bf48a..2e9aa000 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
+++
b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407NullObjectFactory.java
@@ -21,32 +21,25 @@ import
org.apache.commons.pool2.BaseKeyedPooledObjectFactory;
import org.apache.commons.pool2.PooledObject;
import org.apache.commons.pool2.impl.DefaultPooledObject;
-public final class KeyedPool407Factory extends
BaseKeyedPooledObjectFactory<String, KeyedPool407Fixture, RuntimeException> {
+public final class KeyedPool407NullObjectFactory extends
BaseKeyedPooledObjectFactory<String, KeyedPool407Fixture, RuntimeException> {
- private final KeyedPool407Fixture fixture;
-
- KeyedPool407Factory(final KeyedPool407Fixture fixture) {
- this.fixture = fixture;
+ @Override
+ public KeyedPool407Fixture create(final String key) {
+ return null;
}
@Override
- public KeyedPool407Fixture create(final String key) {
- // This is key to the test, creation failed and returns null for
instance see
- //
https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.io.transport.modbus/src/main/java/org/openhab/core/io/transport/modbus/internal/pooling/ModbusSlaveConnectionFactoryImpl.java#L163
- // the test passes when this returns new Pool407Fixture();
- return fixture;
+ public PooledObject<KeyedPool407Fixture> makeObject(final String key)
throws RuntimeException {
+ return new DefaultPooledObject<>(null);
}
@Override
public boolean validateObject(final String key, final
PooledObject<KeyedPool407Fixture> p) {
- // TODO Should this be enough even if wrap() does throw and returns a
DefaultPooledObject wrapping a null?
- return p.getObject() != null;
+ return p != null && p.getObject() != null;
}
@Override
public PooledObject<KeyedPool407Fixture> wrap(final KeyedPool407Fixture
value) {
- // Require a non-null value.
- // return new DefaultPooledObject<>(Objects.requireNonNull(value,
"value"));
- return new DefaultPooledObject<>(value);
+ return new DefaultPooledObject<>(null);
}
}
diff --git
a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407NullPoolableObjectFactory.java
similarity index 54%
rename from
src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
rename to
src/test/java/org/apache/commons/pool2/pool407/KeyedPool407NullPoolableObjectFactory.java
index 950bf48a..1896a18f 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
+++
b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407NullPoolableObjectFactory.java
@@ -19,34 +19,26 @@ package org.apache.commons.pool2.pool407;
import org.apache.commons.pool2.BaseKeyedPooledObjectFactory;
import org.apache.commons.pool2.PooledObject;
-import org.apache.commons.pool2.impl.DefaultPooledObject;
-public final class KeyedPool407Factory extends
BaseKeyedPooledObjectFactory<String, KeyedPool407Fixture, RuntimeException> {
+public final class KeyedPool407NullPoolableObjectFactory extends
BaseKeyedPooledObjectFactory<String, KeyedPool407Fixture, RuntimeException> {
- private final KeyedPool407Fixture fixture;
-
- KeyedPool407Factory(final KeyedPool407Fixture fixture) {
- this.fixture = fixture;
+ @Override
+ public KeyedPool407Fixture create(final String key) {
+ return null;
}
@Override
- public KeyedPool407Fixture create(final String key) {
- // This is key to the test, creation failed and returns null for
instance see
- //
https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.io.transport.modbus/src/main/java/org/openhab/core/io/transport/modbus/internal/pooling/ModbusSlaveConnectionFactoryImpl.java#L163
- // the test passes when this returns new Pool407Fixture();
- return fixture;
+ public PooledObject<KeyedPool407Fixture> makeObject(final String key)
throws RuntimeException {
+ return null;
}
@Override
public boolean validateObject(final String key, final
PooledObject<KeyedPool407Fixture> p) {
- // TODO Should this be enough even if wrap() does throw and returns a
DefaultPooledObject wrapping a null?
- return p.getObject() != null;
+ return p != null && p.getObject() != null;
}
@Override
public PooledObject<KeyedPool407Fixture> wrap(final KeyedPool407Fixture
value) {
- // Require a non-null value.
- // return new DefaultPooledObject<>(Objects.requireNonNull(value,
"value"));
- return new DefaultPooledObject<>(value);
+ return null;
}
}
diff --git
a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Test.java
b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Test.java
index dc54d027..95adba2f 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Test.java
+++ b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Test.java
@@ -23,6 +23,8 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
+import org.apache.commons.pool2.BaseKeyedPooledObjectFactory;
+import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
public class KeyedPool407Test {
@@ -50,9 +52,10 @@ public class KeyedPool407Test {
private static final int POOL_SIZE = 3;
- private void test(final KeyedPool407Fixture fixture, final int poolSize)
throws InterruptedException {
+ private void test(final BaseKeyedPooledObjectFactory<String,
KeyedPool407Fixture, RuntimeException> factory, final int poolSize)
+ throws InterruptedException {
final ExecutorService executor =
Executors.newFixedThreadPool(poolSize);
- final KeyedPool407 pool = new KeyedPool407(fixture);
+ final KeyedPool407 pool = new KeyedPool407(factory);
// start 'poolSize' threads that try to borrow a Pool407Fixture with
the same key
for (int i = 0; i < poolSize; i++) {
@@ -65,12 +68,24 @@ public class KeyedPool407Test {
}
@Test
- public void testFail() throws InterruptedException {
- test(null, POOL_SIZE);
+ public void testNormalFactoryNonNullFixture() throws InterruptedException {
+ test(new KeyedPool407NormalFactory(new KeyedPool407Fixture()), 3);
}
@Test
- public void testPass() throws InterruptedException {
- test(new KeyedPool407Fixture(), 3);
+ public void testNormalFactoryNullFixture() throws InterruptedException {
+ test(new KeyedPool407NormalFactory(null), POOL_SIZE);
+ }
+
+ @Test
+ @Disabled("Either normal to fail or we should handle nulls internally
better.")
+ public void testNullObjectFactory() throws InterruptedException {
+ test(new KeyedPool407NullObjectFactory(), POOL_SIZE);
+ }
+
+ @Test
+ @Disabled("Either normal to fail or we should handle nulls internally
better.")
+ public void testNullPoolableFactory() throws InterruptedException {
+ test(new KeyedPool407NullPoolableObjectFactory(), POOL_SIZE);
}
}
diff --git a/src/test/java/org/apache/commons/pool2/pool407/Pool407.java
b/src/test/java/org/apache/commons/pool2/pool407/Pool407.java
index 54aa8463..d393dfdc 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/Pool407.java
+++ b/src/test/java/org/apache/commons/pool2/pool407/Pool407.java
@@ -17,11 +17,16 @@
package org.apache.commons.pool2.pool407;
+import org.apache.commons.pool2.BasePooledObjectFactory;
import org.apache.commons.pool2.impl.GenericObjectPool;
public final class Pool407 extends GenericObjectPool<Pool407Fixture,
RuntimeException> {
public Pool407(final Pool407Fixture fixture) {
- super(new Pool407Factory(fixture), new Pool407Config());
+ super(new Pool407NormalFactory(fixture), new Pool407Config());
+ }
+
+ public Pool407(final BasePooledObjectFactory<Pool407Fixture,
RuntimeException> factory) {
+ super(factory, new Pool407Config());
}
}
diff --git a/src/test/java/org/apache/commons/pool2/pool407/Pool407Factory.java
b/src/test/java/org/apache/commons/pool2/pool407/Pool407NormalFactory.java
similarity index 92%
copy from src/test/java/org/apache/commons/pool2/pool407/Pool407Factory.java
copy to src/test/java/org/apache/commons/pool2/pool407/Pool407NormalFactory.java
index 11ed07e1..26b89a7c 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/Pool407Factory.java
+++ b/src/test/java/org/apache/commons/pool2/pool407/Pool407NormalFactory.java
@@ -21,11 +21,11 @@ import org.apache.commons.pool2.BasePooledObjectFactory;
import org.apache.commons.pool2.PooledObject;
import org.apache.commons.pool2.impl.DefaultPooledObject;
-public final class Pool407Factory extends
BasePooledObjectFactory<Pool407Fixture, RuntimeException> {
+public final class Pool407NormalFactory extends
BasePooledObjectFactory<Pool407Fixture, RuntimeException> {
private final Pool407Fixture fixture;
- Pool407Factory(final Pool407Fixture fixture) {
+ Pool407NormalFactory(final Pool407Fixture fixture) {
this.fixture = fixture;
}
diff --git a/src/test/java/org/apache/commons/pool2/pool407/Pool407Factory.java
b/src/test/java/org/apache/commons/pool2/pool407/Pool407NullObjectFactory.java
similarity index 57%
rename from src/test/java/org/apache/commons/pool2/pool407/Pool407Factory.java
rename to
src/test/java/org/apache/commons/pool2/pool407/Pool407NullObjectFactory.java
index 11ed07e1..fadcaa03 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/Pool407Factory.java
+++
b/src/test/java/org/apache/commons/pool2/pool407/Pool407NullObjectFactory.java
@@ -21,32 +21,25 @@ import org.apache.commons.pool2.BasePooledObjectFactory;
import org.apache.commons.pool2.PooledObject;
import org.apache.commons.pool2.impl.DefaultPooledObject;
-public final class Pool407Factory extends
BasePooledObjectFactory<Pool407Fixture, RuntimeException> {
+public final class Pool407NullObjectFactory extends
BasePooledObjectFactory<Pool407Fixture, RuntimeException> {
- private final Pool407Fixture fixture;
-
- Pool407Factory(final Pool407Fixture fixture) {
- this.fixture = fixture;
+ @Override
+ public Pool407Fixture create() {
+ return null;
}
@Override
- public Pool407Fixture create() {
- // This is key to the test, creation failed and returns null for
instance see
- //
https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.io.transport.modbus/src/main/java/org/openhab/core/io/transport/modbus/internal/pooling/ModbusSlaveConnectionFactoryImpl.java#L163
- // the test passes when this returns new Pool407Fixture();
- return fixture;
+ public PooledObject<Pool407Fixture> makeObject() throws RuntimeException {
+ return new DefaultPooledObject<>(null);
}
@Override
public boolean validateObject(final PooledObject<Pool407Fixture> p) {
- // TODO Should this be enough even if wrap() does throw and returns a
DefaultPooledObject wrapping a null?
- return p.getObject() != null;
+ return p != null && p.getObject() != null;
}
@Override
public PooledObject<Pool407Fixture> wrap(final Pool407Fixture value) {
- // Require a non-null value.
- // return new DefaultPooledObject<>(Objects.requireNonNull(value,
"value"));
- return new DefaultPooledObject<>(value);
+ return new DefaultPooledObject<>(null);
}
}
diff --git a/src/test/java/org/apache/commons/pool2/pool407/Pool407.java
b/src/test/java/org/apache/commons/pool2/pool407/Pool407NullPoolableObjectFactory.java
similarity index 55%
copy from src/test/java/org/apache/commons/pool2/pool407/Pool407.java
copy to
src/test/java/org/apache/commons/pool2/pool407/Pool407NullPoolableObjectFactory.java
index 54aa8463..b17d0e57 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/Pool407.java
+++
b/src/test/java/org/apache/commons/pool2/pool407/Pool407NullPoolableObjectFactory.java
@@ -17,11 +17,28 @@
package org.apache.commons.pool2.pool407;
-import org.apache.commons.pool2.impl.GenericObjectPool;
+import org.apache.commons.pool2.BasePooledObjectFactory;
+import org.apache.commons.pool2.PooledObject;
-public final class Pool407 extends GenericObjectPool<Pool407Fixture,
RuntimeException> {
+public final class Pool407NullPoolableObjectFactory extends
BasePooledObjectFactory<Pool407Fixture, RuntimeException> {
- public Pool407(final Pool407Fixture fixture) {
- super(new Pool407Factory(fixture), new Pool407Config());
+ @Override
+ public Pool407Fixture create() {
+ return null;
+ }
+
+ @Override
+ public PooledObject<Pool407Fixture> makeObject() throws RuntimeException {
+ return null;
+ }
+
+ @Override
+ public boolean validateObject(final PooledObject<Pool407Fixture> p) {
+ return p != null && p.getObject() != null;
+ }
+
+ @Override
+ public PooledObject<Pool407Fixture> wrap(final Pool407Fixture value) {
+ return null;
}
}
diff --git a/src/test/java/org/apache/commons/pool2/pool407/Pool407Test.java
b/src/test/java/org/apache/commons/pool2/pool407/Pool407Test.java
index bb1675e3..ccaa5569 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/Pool407Test.java
+++ b/src/test/java/org/apache/commons/pool2/pool407/Pool407Test.java
@@ -23,6 +23,8 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
+import org.apache.commons.pool2.BasePooledObjectFactory;
+import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
public class Pool407Test {
@@ -49,9 +51,9 @@ public class Pool407Test {
private static final int POOL_SIZE = 3;
- private void test(final Pool407Fixture fixture, final int poolSize) throws
InterruptedException {
+ private void test(final BasePooledObjectFactory<Pool407Fixture,
RuntimeException> factory, final int poolSize) throws InterruptedException {
final ExecutorService executor =
Executors.newFixedThreadPool(poolSize);
- final Pool407 pool = new Pool407(fixture);
+ final Pool407 pool = new Pool407(factory);
// start 'poolSize' threads that try to borrow a Pool407Fixture with
the same key
for (int i = 0; i < poolSize; i++) {
@@ -64,12 +66,24 @@ public class Pool407Test {
}
@Test
- public void testFail() throws InterruptedException {
- test(null, POOL_SIZE);
+ public void testNormalFactoryNonNullFixture() throws InterruptedException {
+ test(new Pool407NormalFactory(new Pool407Fixture()), 3);
}
@Test
- public void testPass() throws InterruptedException {
- test(new Pool407Fixture(), 3);
+ public void testNormalFactoryNullFixture() throws InterruptedException {
+ test(new Pool407NormalFactory(null), POOL_SIZE);
+ }
+
+ @Test
+ @Disabled("Either normal to fail or we should handle nulls internally
better.")
+ public void testNullObjectFactory() throws InterruptedException {
+ test(new Pool407NullObjectFactory(), POOL_SIZE);
+ }
+
+ @Test
+ @Disabled("Either normal to fail or we should handle nulls internally
better.")
+ public void testNullPoolableFactory() throws InterruptedException {
+ test(new Pool407NullPoolableObjectFactory(), POOL_SIZE);
}
}
