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 b73c2cd5 [POOL-407] Threads get stuck when idleObjects list is empty
b73c2cd5 is described below
commit b73c2cd56384569bbec268990ce3a1a48e7acf6f
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Fri Oct 7 20:11:46 2022 -0400
[POOL-407] Threads get stuck when idleObjects list is empty
Add working test case for non-keyed version of the code
---
.../pool2/BaseKeyedPooledObjectFactory.java | 4 +-
.../commons/pool2/BasePooledObjectFactory.java | 4 +-
.../commons/pool2/pool407/KeyedPool407Factory.java | 5 +-
.../org/apache/commons/pool2/pool407/Pool407.java | 27 ++++++++
.../commons/pool2/pool407/Pool407Config.java | 31 +++++++++
...eyedPool407Factory.java => Pool407Factory.java} | 19 +++---
.../commons/pool2/pool407/Pool407Fixture.java | 22 +++++++
.../apache/commons/pool2/pool407/Pool407Test.java | 75 ++++++++++++++++++++++
8 files changed, 172 insertions(+), 15 deletions(-)
diff --git
a/src/main/java/org/apache/commons/pool2/BaseKeyedPooledObjectFactory.java
b/src/main/java/org/apache/commons/pool2/BaseKeyedPooledObjectFactory.java
index 369550b1..0c894101 100644
--- a/src/main/java/org/apache/commons/pool2/BaseKeyedPooledObjectFactory.java
+++ b/src/main/java/org/apache/commons/pool2/BaseKeyedPooledObjectFactory.java
@@ -16,6 +16,8 @@
*/
package org.apache.commons.pool2;
+import java.util.Objects;
+
/**
* A base implementation of {@code KeyedPooledObjectFactory}.
* <p>
@@ -76,7 +78,7 @@ public abstract class BaseKeyedPooledObjectFactory<K, V, E
extends Exception> ex
@Override
public PooledObject<V> makeObject(final K key) throws E {
- return wrap(create(key));
+ return wrap(Objects.requireNonNull(create(key),
"BaseKeyedPooledObjectFactory.create(key)"));
}
/**
diff --git
a/src/main/java/org/apache/commons/pool2/BasePooledObjectFactory.java
b/src/main/java/org/apache/commons/pool2/BasePooledObjectFactory.java
index d7ae8de6..da8c456b 100644
--- a/src/main/java/org/apache/commons/pool2/BasePooledObjectFactory.java
+++ b/src/main/java/org/apache/commons/pool2/BasePooledObjectFactory.java
@@ -16,6 +16,8 @@
*/
package org.apache.commons.pool2;
+import java.util.Objects;
+
/**
* A base implementation of {@code PoolableObjectFactory}.
* <p>
@@ -68,7 +70,7 @@ public abstract class BasePooledObjectFactory<T, E extends
Exception> extends Ba
@Override
public PooledObject<T> makeObject() throws E {
- return wrap(create());
+ return wrap(Objects.requireNonNull(create(),
"BasePooledObjectFactory.create()"));
}
/**
diff --git
a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
index 07db0a88..950bf48a 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
+++ b/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
@@ -17,8 +17,6 @@
package org.apache.commons.pool2.pool407;
-import java.util.Objects;
-
import org.apache.commons.pool2.BaseKeyedPooledObjectFactory;
import org.apache.commons.pool2.PooledObject;
import org.apache.commons.pool2.impl.DefaultPooledObject;
@@ -48,6 +46,7 @@ public final class KeyedPool407Factory extends
BaseKeyedPooledObjectFactory<Stri
@Override
public PooledObject<KeyedPool407Fixture> wrap(final KeyedPool407Fixture
value) {
// Require a non-null value.
- return new DefaultPooledObject<>(Objects.requireNonNull(value,
"value"));
+ // return new DefaultPooledObject<>(Objects.requireNonNull(value,
"value"));
+ return new DefaultPooledObject<>(value);
}
}
diff --git a/src/test/java/org/apache/commons/pool2/pool407/Pool407.java
b/src/test/java/org/apache/commons/pool2/pool407/Pool407.java
new file mode 100644
index 00000000..54aa8463
--- /dev/null
+++ b/src/test/java/org/apache/commons/pool2/pool407/Pool407.java
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.pool2.pool407;
+
+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());
+ }
+}
diff --git a/src/test/java/org/apache/commons/pool2/pool407/Pool407Config.java
b/src/test/java/org/apache/commons/pool2/pool407/Pool407Config.java
new file mode 100644
index 00000000..8cbbe2f7
--- /dev/null
+++ b/src/test/java/org/apache/commons/pool2/pool407/Pool407Config.java
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.pool2.pool407;
+
+import java.time.Duration;
+
+import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+
+public final class Pool407Config extends
GenericObjectPoolConfig<Pool407Fixture> {
+
+ public Pool407Config() {
+ setBlockWhenExhausted(true);
+ setMaxTotal(1);
+ setMaxWait(Duration.ofMillis(-1));
+ }
+}
diff --git
a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
b/src/test/java/org/apache/commons/pool2/pool407/Pool407Factory.java
similarity index 71%
copy from
src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
copy to src/test/java/org/apache/commons/pool2/pool407/Pool407Factory.java
index 07db0a88..11ed07e1 100644
--- a/src/test/java/org/apache/commons/pool2/pool407/KeyedPool407Factory.java
+++ b/src/test/java/org/apache/commons/pool2/pool407/Pool407Factory.java
@@ -17,22 +17,20 @@
package org.apache.commons.pool2.pool407;
-import java.util.Objects;
-
-import org.apache.commons.pool2.BaseKeyedPooledObjectFactory;
+import org.apache.commons.pool2.BasePooledObjectFactory;
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 Pool407Factory extends
BasePooledObjectFactory<Pool407Fixture, RuntimeException> {
- private final KeyedPool407Fixture fixture;
+ private final Pool407Fixture fixture;
- KeyedPool407Factory(final KeyedPool407Fixture fixture) {
+ Pool407Factory(final Pool407Fixture fixture) {
this.fixture = fixture;
}
@Override
- public KeyedPool407Fixture create(final String key) {
+ 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();
@@ -40,14 +38,15 @@ public final class KeyedPool407Factory extends
BaseKeyedPooledObjectFactory<Stri
}
@Override
- public boolean validateObject(final String key, final
PooledObject<KeyedPool407Fixture> p) {
+ 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;
}
@Override
- public PooledObject<KeyedPool407Fixture> wrap(final KeyedPool407Fixture
value) {
+ public PooledObject<Pool407Fixture> wrap(final Pool407Fixture value) {
// Require a non-null value.
- return new DefaultPooledObject<>(Objects.requireNonNull(value,
"value"));
+ // return new DefaultPooledObject<>(Objects.requireNonNull(value,
"value"));
+ return new DefaultPooledObject<>(value);
}
}
diff --git a/src/test/java/org/apache/commons/pool2/pool407/Pool407Fixture.java
b/src/test/java/org/apache/commons/pool2/pool407/Pool407Fixture.java
new file mode 100644
index 00000000..0488bb22
--- /dev/null
+++ b/src/test/java/org/apache/commons/pool2/pool407/Pool407Fixture.java
@@ -0,0 +1,22 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.pool2.pool407;
+
+public final class Pool407Fixture {
+ // empty
+}
diff --git a/src/test/java/org/apache/commons/pool2/pool407/Pool407Test.java
b/src/test/java/org/apache/commons/pool2/pool407/Pool407Test.java
new file mode 100644
index 00000000..bb1675e3
--- /dev/null
+++ b/src/test/java/org/apache/commons/pool2/pool407/Pool407Test.java
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.pool2.pool407;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.jupiter.api.Test;
+
+public class Pool407Test {
+
+ private static class Pool407Borrower implements Runnable {
+ private final Pool407 pool;
+
+ public Pool407Borrower(final Pool407 pool) {
+ this.pool = pool;
+ }
+
+ @Override
+ public void run() {
+ try {
+ final Pool407Fixture foo = pool.borrowObject();
+ if (foo != null) {
+ pool.returnObject(foo);
+ }
+ } catch (final Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+
+ private static final int POOL_SIZE = 3;
+
+ private void test(final Pool407Fixture fixture, final int poolSize) throws
InterruptedException {
+ final ExecutorService executor =
Executors.newFixedThreadPool(poolSize);
+ final Pool407 pool = new Pool407(fixture);
+
+ // start 'poolSize' threads that try to borrow a Pool407Fixture with
the same key
+ for (int i = 0; i < poolSize; i++) {
+ executor.execute(new Pool407Borrower(pool));
+ }
+
+ // this never finishes because two threads are stuck forever
+ executor.shutdown();
+ assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS));
+ }
+
+ @Test
+ public void testFail() throws InterruptedException {
+ test(null, POOL_SIZE);
+ }
+
+ @Test
+ public void testPass() throws InterruptedException {
+ test(new Pool407Fixture(), 3);
+ }
+}
