Re: Regression in Pool 2.13.0
RC vote is out: https://lists.apache.org/thread/fcjo1yt47m69yp1r1fmx858c31q18rp0 Gary On Mon, Dec 29, 2025 at 3:49 PM Gary Gregory wrote: > > On Mon, Dec 29, 2025 at 2:55 PM Phil Steitz wrote: > > > > Sorry, that test should have been disabled. The root cause of it is the > > race condition in POOL-413, which is still open. > > OK, I've disabled that test and will restart teh RC... > > Gary > > > > > On Mon, Dec 29, 2025 at 6:19 AM Gary Gregory wrote: > > > > > On Mon, Dec 29, 2025 at 8:06 AM Gary Gregory > > > wrote: > > > > > > > > On Sun, Dec 28, 2025 at 12:44 PM Phil Steitz > > > wrote: > > > > > > > > > > I would say yes, get the regression fix out now and work on POOL-413 > > > et al > > > > > afterwards. > > > > > > > > Starting RC... > > > > > > Hm, I just got this random error on my 3rd local build: > > > > > > [INFO] Running org.apache.commons.pool2.impl.TestGenericObjectPool > > > [ERROR] Tests run: 110, Failures: 1, Errors: 0, Skipped: 1, Time > > > elapsed: 114.3 s <<< FAILURE! -- in > > > org.apache.commons.pool2.impl.TestGenericObjectPool > > > [ERROR] > > > org.apache.commons.pool2.impl.TestGenericObjectPool.testAddObjectConcurrentCallsRespectsMaxIdle > > > -- Time elapsed: 0.013 s <<< FAILURE! > > > org.opentest4j.AssertionFailedError: Concurrent addObject() calls > > > should not exceed maxIdle limit of 5, but found 7 idle objects ==> > > > expected: but was: > > > at > > > org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) > > > at > > > org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) > > > at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63) > > > at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36) > > > at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:214) > > > at > > > org.apache.commons.pool2.impl.TestGenericObjectPool.testAddObjectConcurrentCallsRespectsMaxIdle(TestGenericObjectPool.java:1033) > > > at java.base/java.lang.reflect.Method.invoke(Method.java:569) > > > at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) > > > at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) > > > > > > Any thoughts? > > > > > > Gary > > > > > > > > > > > Gary > > > > > > > > > > > > > > Phil > > > > > > > > > > On Sun, Dec 28, 2025 at 6:04 AM Gary Gregory > > > wrote: > > > > > > > > > > > Hi Phil and all: > > > > > > > > > > > > If I apply https://github.com/apache/commons-pool/pull/453.diff > > > > > > locally, running the test from Eclipse fails, and a Maven (default > > > > > > goal) build fails on random repetition items, for example: > > > > > > > > > https://gist.github.com/garydgregory/fdfbb61360813326998c0e253b10ed52 > > > > > > > > > > > > I assume that we want to proceed with an RC to address the > > > regression, > > > > > > and we are pushing this out to future work. > > > > > > > > > > > > Gary > > > > > > > > > > > > On Sun, Dec 28, 2025 at 7:37 AM Gary Gregory > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Dec 27, 2025, 18:09 Phil Steitz > > > wrote: > > > > > > >> > > > > > > >> I updated the changelog and I think we are ready for a patch > > > release. > > > > > > > > > > > > > > > > > > > > > OK sounds good. I'll get to that today. > > > > > > > > > > > > > > Gary > > > > > > > > > > > > > >> > > > > > > >> Phil > > > > > > >> > > > > > > >> On Sat, Dec 27, 2025 at 12:24 PM Gary Gregory < > > > [email protected]> > > > > > > >> wrote: > > > > > > >> > > > > > > >> > That all sounds good. I can create a release candidate anytime > > > if you > > > > > > want. > > > > > > >> > > > > > > > >> > Gary > > > > > > >> > > > > > > > >> > On Sat, Dec 27, 2025 at 1:43 PM Phil Steitz < > > > [email protected]> > > > > > > wrote: > > > > > > >> > > > > > > > > >> > > Given that the regression reported in POOL-427 is > > > significant, I > > > > > > think we > > > > > > >> > > should move quickly to validate the fix for the regression > > > > > > >> > > (or > > > > > > revert > > > > > > >> > back > > > > > > >> > > to the previous version of the method) and create a patch > > > release > > > > > > as soon > > > > > > >> > > as possible. The investigations around POOL-413 are great > > > > > > >> > > and > > > > > > should > > > > > > >> > > continue in parallel. It would be great if we could discuss > > > ideas > > > > > > for > > > > > > >> > how > > > > > > >> > > to address the core issue there here instead of spread across > > > PRs. > > > > > > >> > > > > > > > > >> > > Phil > > > > > > >> > > > > > > > > >> > > On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz < > > > [email protected] > > > > > > > > > > > > > >> > wrote: > > > > > > >> > > > > > > > > >> > > > I just reverted the added sync in PR #452, which violates > > > the "no > > > > > > >> > factory > > > > > > >> > > > methods while holding locks" invariant. Strangely, the > > > added > > > > > > tests f
Re: Regression in Pool 2.13.0
On Mon, Dec 29, 2025 at 2:55 PM Phil Steitz wrote: > > Sorry, that test should have been disabled. The root cause of it is the > race condition in POOL-413, which is still open. OK, I've disabled that test and will restart teh RC... Gary > > On Mon, Dec 29, 2025 at 6:19 AM Gary Gregory wrote: > > > On Mon, Dec 29, 2025 at 8:06 AM Gary Gregory > > wrote: > > > > > > On Sun, Dec 28, 2025 at 12:44 PM Phil Steitz > > wrote: > > > > > > > > I would say yes, get the regression fix out now and work on POOL-413 > > et al > > > > afterwards. > > > > > > Starting RC... > > > > Hm, I just got this random error on my 3rd local build: > > > > [INFO] Running org.apache.commons.pool2.impl.TestGenericObjectPool > > [ERROR] Tests run: 110, Failures: 1, Errors: 0, Skipped: 1, Time > > elapsed: 114.3 s <<< FAILURE! -- in > > org.apache.commons.pool2.impl.TestGenericObjectPool > > [ERROR] > > org.apache.commons.pool2.impl.TestGenericObjectPool.testAddObjectConcurrentCallsRespectsMaxIdle > > -- Time elapsed: 0.013 s <<< FAILURE! > > org.opentest4j.AssertionFailedError: Concurrent addObject() calls > > should not exceed maxIdle limit of 5, but found 7 idle objects ==> > > expected: but was: > > at > > org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) > > at > > org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) > > at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63) > > at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36) > > at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:214) > > at > > org.apache.commons.pool2.impl.TestGenericObjectPool.testAddObjectConcurrentCallsRespectsMaxIdle(TestGenericObjectPool.java:1033) > > at java.base/java.lang.reflect.Method.invoke(Method.java:569) > > at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) > > at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) > > > > Any thoughts? > > > > Gary > > > > > > > > Gary > > > > > > > > > > > Phil > > > > > > > > On Sun, Dec 28, 2025 at 6:04 AM Gary Gregory > > wrote: > > > > > > > > > Hi Phil and all: > > > > > > > > > > If I apply https://github.com/apache/commons-pool/pull/453.diff > > > > > locally, running the test from Eclipse fails, and a Maven (default > > > > > goal) build fails on random repetition items, for example: > > > > > > > https://gist.github.com/garydgregory/fdfbb61360813326998c0e253b10ed52 > > > > > > > > > > I assume that we want to proceed with an RC to address the > > regression, > > > > > and we are pushing this out to future work. > > > > > > > > > > Gary > > > > > > > > > > On Sun, Dec 28, 2025 at 7:37 AM Gary Gregory > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Dec 27, 2025, 18:09 Phil Steitz > > wrote: > > > > > >> > > > > > >> I updated the changelog and I think we are ready for a patch > > release. > > > > > > > > > > > > > > > > > > OK sounds good. I'll get to that today. > > > > > > > > > > > > Gary > > > > > > > > > > > >> > > > > > >> Phil > > > > > >> > > > > > >> On Sat, Dec 27, 2025 at 12:24 PM Gary Gregory < > > [email protected]> > > > > > >> wrote: > > > > > >> > > > > > >> > That all sounds good. I can create a release candidate anytime > > if you > > > > > want. > > > > > >> > > > > > > >> > Gary > > > > > >> > > > > > > >> > On Sat, Dec 27, 2025 at 1:43 PM Phil Steitz < > > [email protected]> > > > > > wrote: > > > > > >> > > > > > > > >> > > Given that the regression reported in POOL-427 is > > significant, I > > > > > think we > > > > > >> > > should move quickly to validate the fix for the regression (or > > > > > revert > > > > > >> > back > > > > > >> > > to the previous version of the method) and create a patch > > release > > > > > as soon > > > > > >> > > as possible. The investigations around POOL-413 are great and > > > > > should > > > > > >> > > continue in parallel. It would be great if we could discuss > > ideas > > > > > for > > > > > >> > how > > > > > >> > > to address the core issue there here instead of spread across > > PRs. > > > > > >> > > > > > > > >> > > Phil > > > > > >> > > > > > > > >> > > On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz < > > [email protected] > > > > > > > > > > > >> > wrote: > > > > > >> > > > > > > > >> > > > I just reverted the added sync in PR #452, which violates > > the "no > > > > > >> > factory > > > > > >> > > > methods while holding locks" invariant. Strangely, the > > added > > > > > tests for > > > > > >> > > > POOL-426 still pass. I think the race condition is still > > present > > > > > and > > > > > >> > the > > > > > >> > > > general problem in POOL-413 remains unresolved. > > > > > >> > > > > > > > > >> > > > On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz < > > > > > [email protected]> > > > > > >> > wrote: > > > > > >> > > > > > > > > >> > > >> The fix for POOL-425 included in the 2.13.0 release > > introduced a > > > > > >> > > >> regre
Re: Regression in Pool 2.13.0
Sorry, that test should have been disabled. The root cause of it is the race condition in POOL-413, which is still open. On Mon, Dec 29, 2025 at 6:19 AM Gary Gregory wrote: > On Mon, Dec 29, 2025 at 8:06 AM Gary Gregory > wrote: > > > > On Sun, Dec 28, 2025 at 12:44 PM Phil Steitz > wrote: > > > > > > I would say yes, get the regression fix out now and work on POOL-413 > et al > > > afterwards. > > > > Starting RC... > > Hm, I just got this random error on my 3rd local build: > > [INFO] Running org.apache.commons.pool2.impl.TestGenericObjectPool > [ERROR] Tests run: 110, Failures: 1, Errors: 0, Skipped: 1, Time > elapsed: 114.3 s <<< FAILURE! -- in > org.apache.commons.pool2.impl.TestGenericObjectPool > [ERROR] > org.apache.commons.pool2.impl.TestGenericObjectPool.testAddObjectConcurrentCallsRespectsMaxIdle > -- Time elapsed: 0.013 s <<< FAILURE! > org.opentest4j.AssertionFailedError: Concurrent addObject() calls > should not exceed maxIdle limit of 5, but found 7 idle objects ==> > expected: but was: > at > org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) > at > org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) > at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63) > at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36) > at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:214) > at > org.apache.commons.pool2.impl.TestGenericObjectPool.testAddObjectConcurrentCallsRespectsMaxIdle(TestGenericObjectPool.java:1033) > at java.base/java.lang.reflect.Method.invoke(Method.java:569) > at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) > at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) > > Any thoughts? > > Gary > > > > > Gary > > > > > > > > Phil > > > > > > On Sun, Dec 28, 2025 at 6:04 AM Gary Gregory > wrote: > > > > > > > Hi Phil and all: > > > > > > > > If I apply https://github.com/apache/commons-pool/pull/453.diff > > > > locally, running the test from Eclipse fails, and a Maven (default > > > > goal) build fails on random repetition items, for example: > > > > > https://gist.github.com/garydgregory/fdfbb61360813326998c0e253b10ed52 > > > > > > > > I assume that we want to proceed with an RC to address the > regression, > > > > and we are pushing this out to future work. > > > > > > > > Gary > > > > > > > > On Sun, Dec 28, 2025 at 7:37 AM Gary Gregory > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Dec 27, 2025, 18:09 Phil Steitz > wrote: > > > > >> > > > > >> I updated the changelog and I think we are ready for a patch > release. > > > > > > > > > > > > > > > OK sounds good. I'll get to that today. > > > > > > > > > > Gary > > > > > > > > > >> > > > > >> Phil > > > > >> > > > > >> On Sat, Dec 27, 2025 at 12:24 PM Gary Gregory < > [email protected]> > > > > >> wrote: > > > > >> > > > > >> > That all sounds good. I can create a release candidate anytime > if you > > > > want. > > > > >> > > > > > >> > Gary > > > > >> > > > > > >> > On Sat, Dec 27, 2025 at 1:43 PM Phil Steitz < > [email protected]> > > > > wrote: > > > > >> > > > > > > >> > > Given that the regression reported in POOL-427 is > significant, I > > > > think we > > > > >> > > should move quickly to validate the fix for the regression (or > > > > revert > > > > >> > back > > > > >> > > to the previous version of the method) and create a patch > release > > > > as soon > > > > >> > > as possible. The investigations around POOL-413 are great and > > > > should > > > > >> > > continue in parallel. It would be great if we could discuss > ideas > > > > for > > > > >> > how > > > > >> > > to address the core issue there here instead of spread across > PRs. > > > > >> > > > > > > >> > > Phil > > > > >> > > > > > > >> > > On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz < > [email protected] > > > > > > > > > >> > wrote: > > > > >> > > > > > > >> > > > I just reverted the added sync in PR #452, which violates > the "no > > > > >> > factory > > > > >> > > > methods while holding locks" invariant. Strangely, the > added > > > > tests for > > > > >> > > > POOL-426 still pass. I think the race condition is still > present > > > > and > > > > >> > the > > > > >> > > > general problem in POOL-413 remains unresolved. > > > > >> > > > > > > > >> > > > On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz < > > > > [email protected]> > > > > >> > wrote: > > > > >> > > > > > > > >> > > >> The fix for POOL-425 included in the 2.13.0 release > introduced a > > > > >> > > >> regression that makes addObject no-op when maxIdle is set > to a > > > > >> > negative > > > > >> > > >> value (no limit). The POOL-425 fix also failed to account > for a > > > > race > > > > >> > > >> condition reported in POOL-426. > > > > >> > > >> > > > > >> > > >> I have created a PR > > > > https://github.com/apache/commons-pool/pull/452 > > > > >> > that addresses > > > > >> > > >> both issues. To avoid
Re: Regression in Pool 2.13.0
On Mon, Dec 29, 2025 at 8:06 AM Gary Gregory wrote: > > On Sun, Dec 28, 2025 at 12:44 PM Phil Steitz wrote: > > > > I would say yes, get the regression fix out now and work on POOL-413 et al > > afterwards. > > Starting RC... Hm, I just got this random error on my 3rd local build: [INFO] Running org.apache.commons.pool2.impl.TestGenericObjectPool [ERROR] Tests run: 110, Failures: 1, Errors: 0, Skipped: 1, Time elapsed: 114.3 s <<< FAILURE! -- in org.apache.commons.pool2.impl.TestGenericObjectPool [ERROR] org.apache.commons.pool2.impl.TestGenericObjectPool.testAddObjectConcurrentCallsRespectsMaxIdle -- Time elapsed: 0.013 s <<< FAILURE! org.opentest4j.AssertionFailedError: Concurrent addObject() calls should not exceed maxIdle limit of 5, but found 7 idle objects ==> expected: but was: at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151) at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132) at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63) at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36) at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:214) at org.apache.commons.pool2.impl.TestGenericObjectPool.testAddObjectConcurrentCallsRespectsMaxIdle(TestGenericObjectPool.java:1033) at java.base/java.lang.reflect.Method.invoke(Method.java:569) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) Any thoughts? Gary > > Gary > > > > > Phil > > > > On Sun, Dec 28, 2025 at 6:04 AM Gary Gregory wrote: > > > > > Hi Phil and all: > > > > > > If I apply https://github.com/apache/commons-pool/pull/453.diff > > > locally, running the test from Eclipse fails, and a Maven (default > > > goal) build fails on random repetition items, for example: > > > https://gist.github.com/garydgregory/fdfbb61360813326998c0e253b10ed52 > > > > > > I assume that we want to proceed with an RC to address the regression, > > > and we are pushing this out to future work. > > > > > > Gary > > > > > > On Sun, Dec 28, 2025 at 7:37 AM Gary Gregory > > > wrote: > > > > > > > > > > > > > > > > > > > > On Sat, Dec 27, 2025, 18:09 Phil Steitz wrote: > > > >> > > > >> I updated the changelog and I think we are ready for a patch release. > > > > > > > > > > > > OK sounds good. I'll get to that today. > > > > > > > > Gary > > > > > > > >> > > > >> Phil > > > >> > > > >> On Sat, Dec 27, 2025 at 12:24 PM Gary Gregory > > > >> wrote: > > > >> > > > >> > That all sounds good. I can create a release candidate anytime if you > > > want. > > > >> > > > > >> > Gary > > > >> > > > > >> > On Sat, Dec 27, 2025 at 1:43 PM Phil Steitz > > > wrote: > > > >> > > > > > >> > > Given that the regression reported in POOL-427 is significant, I > > > think we > > > >> > > should move quickly to validate the fix for the regression (or > > > revert > > > >> > back > > > >> > > to the previous version of the method) and create a patch release > > > as soon > > > >> > > as possible. The investigations around POOL-413 are great and > > > should > > > >> > > continue in parallel. It would be great if we could discuss ideas > > > for > > > >> > how > > > >> > > to address the core issue there here instead of spread across PRs. > > > >> > > > > > >> > > Phil > > > >> > > > > > >> > > On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz > > > > > > >> > wrote: > > > >> > > > > > >> > > > I just reverted the added sync in PR #452, which violates the "no > > > >> > factory > > > >> > > > methods while holding locks" invariant. Strangely, the added > > > tests for > > > >> > > > POOL-426 still pass. I think the race condition is still present > > > and > > > >> > the > > > >> > > > general problem in POOL-413 remains unresolved. > > > >> > > > > > > >> > > > On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz < > > > [email protected]> > > > >> > wrote: > > > >> > > > > > > >> > > >> The fix for POOL-425 included in the 2.13.0 release introduced a > > > >> > > >> regression that makes addObject no-op when maxIdle is set to a > > > >> > negative > > > >> > > >> value (no limit). The POOL-425 fix also failed to account for a > > > race > > > >> > > >> condition reported in POOL-426. > > > >> > > >> > > > >> > > >> I have created a PR > > > https://github.com/apache/commons-pool/pull/452 > > > >> > that addresses > > > >> > > >> both issues. To avoid the race condition, I had to add > > > >> > synchronization to > > > >> > > >> addObject. I tried several ways to avoid the race by modifying > > > >> > create (as > > > >> > > >> suggested by Raju Gupta, the OP for POOL-426) but I could not > > > find a > > > >> > way to > > > >> > > >> do that safely without introducing other issues. I don't see > > > >> > > >> the > > > >> > added > > > >> > > >> sync in addObject as critical as this method is not used in hot > > > code > > > >> > paths > > > >> > > >> internally and the lock that it acqu
Re: Regression in Pool 2.13.0
On Sun, Dec 28, 2025 at 12:44 PM Phil Steitz wrote: > > I would say yes, get the regression fix out now and work on POOL-413 et al > afterwards. Starting RC... Gary > > Phil > > On Sun, Dec 28, 2025 at 6:04 AM Gary Gregory wrote: > > > Hi Phil and all: > > > > If I apply https://github.com/apache/commons-pool/pull/453.diff > > locally, running the test from Eclipse fails, and a Maven (default > > goal) build fails on random repetition items, for example: > > https://gist.github.com/garydgregory/fdfbb61360813326998c0e253b10ed52 > > > > I assume that we want to proceed with an RC to address the regression, > > and we are pushing this out to future work. > > > > Gary > > > > On Sun, Dec 28, 2025 at 7:37 AM Gary Gregory > > wrote: > > > > > > > > > > > > > > > On Sat, Dec 27, 2025, 18:09 Phil Steitz wrote: > > >> > > >> I updated the changelog and I think we are ready for a patch release. > > > > > > > > > OK sounds good. I'll get to that today. > > > > > > Gary > > > > > >> > > >> Phil > > >> > > >> On Sat, Dec 27, 2025 at 12:24 PM Gary Gregory > > >> wrote: > > >> > > >> > That all sounds good. I can create a release candidate anytime if you > > want. > > >> > > > >> > Gary > > >> > > > >> > On Sat, Dec 27, 2025 at 1:43 PM Phil Steitz > > wrote: > > >> > > > > >> > > Given that the regression reported in POOL-427 is significant, I > > think we > > >> > > should move quickly to validate the fix for the regression (or > > revert > > >> > back > > >> > > to the previous version of the method) and create a patch release > > as soon > > >> > > as possible. The investigations around POOL-413 are great and > > should > > >> > > continue in parallel. It would be great if we could discuss ideas > > for > > >> > how > > >> > > to address the core issue there here instead of spread across PRs. > > >> > > > > >> > > Phil > > >> > > > > >> > > On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz > > > > >> > wrote: > > >> > > > > >> > > > I just reverted the added sync in PR #452, which violates the "no > > >> > factory > > >> > > > methods while holding locks" invariant. Strangely, the added > > tests for > > >> > > > POOL-426 still pass. I think the race condition is still present > > and > > >> > the > > >> > > > general problem in POOL-413 remains unresolved. > > >> > > > > > >> > > > On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz < > > [email protected]> > > >> > wrote: > > >> > > > > > >> > > >> The fix for POOL-425 included in the 2.13.0 release introduced a > > >> > > >> regression that makes addObject no-op when maxIdle is set to a > > >> > negative > > >> > > >> value (no limit). The POOL-425 fix also failed to account for a > > race > > >> > > >> condition reported in POOL-426. > > >> > > >> > > >> > > >> I have created a PR > > https://github.com/apache/commons-pool/pull/452 > > >> > that addresses > > >> > > >> both issues. To avoid the race condition, I had to add > > >> > synchronization to > > >> > > >> addObject. I tried several ways to avoid the race by modifying > > >> > create (as > > >> > > >> suggested by Raju Gupta, the OP for POOL-426) but I could not > > find a > > >> > way to > > >> > > >> do that safely without introducing other issues. I don't see the > > >> > added > > >> > > >> sync in addObject as critical as this method is not used in hot > > code > > >> > paths > > >> > > >> internally and the lock that it acquires is the same lock that > > create > > >> > will > > >> > > >> subsequently acquire if it proceeds to add an object. > > >> > > >> > > >> > > >> The regression could be addressed in a simpler way by just > > fixing the > > >> > > >> error in the code (failure to check for negative maxIdle). If > > there > > >> > are > > >> > > >> any doubts about the PR above, I am happy to make that simple > > >> > change. In > > >> > > >> any case, we should patch this quickly as it will likely break > > some > > >> > apps > > >> > > >> that use addObject with maxIdle unilimited. > > >> > > >> > > >> > > >> Thanks, all, and sorry for my mistake in the POOL-425 fix. > > >> > > >> > > >> > > >> Phil > > >> > > >> > > >> > > > > > >> > > > >> > - > > >> > To unsubscribe, e-mail: [email protected] > > >> > For additional commands, e-mail: [email protected] > > >> > > > >> > > > > > - > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: Regression in Pool 2.13.0
I would say yes, get the regression fix out now and work on POOL-413 et al afterwards. Phil On Sun, Dec 28, 2025 at 6:04 AM Gary Gregory wrote: > Hi Phil and all: > > If I apply https://github.com/apache/commons-pool/pull/453.diff > locally, running the test from Eclipse fails, and a Maven (default > goal) build fails on random repetition items, for example: > https://gist.github.com/garydgregory/fdfbb61360813326998c0e253b10ed52 > > I assume that we want to proceed with an RC to address the regression, > and we are pushing this out to future work. > > Gary > > On Sun, Dec 28, 2025 at 7:37 AM Gary Gregory > wrote: > > > > > > > > > > On Sat, Dec 27, 2025, 18:09 Phil Steitz wrote: > >> > >> I updated the changelog and I think we are ready for a patch release. > > > > > > OK sounds good. I'll get to that today. > > > > Gary > > > >> > >> Phil > >> > >> On Sat, Dec 27, 2025 at 12:24 PM Gary Gregory > >> wrote: > >> > >> > That all sounds good. I can create a release candidate anytime if you > want. > >> > > >> > Gary > >> > > >> > On Sat, Dec 27, 2025 at 1:43 PM Phil Steitz > wrote: > >> > > > >> > > Given that the regression reported in POOL-427 is significant, I > think we > >> > > should move quickly to validate the fix for the regression (or > revert > >> > back > >> > > to the previous version of the method) and create a patch release > as soon > >> > > as possible. The investigations around POOL-413 are great and > should > >> > > continue in parallel. It would be great if we could discuss ideas > for > >> > how > >> > > to address the core issue there here instead of spread across PRs. > >> > > > >> > > Phil > >> > > > >> > > On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz > > >> > wrote: > >> > > > >> > > > I just reverted the added sync in PR #452, which violates the "no > >> > factory > >> > > > methods while holding locks" invariant. Strangely, the added > tests for > >> > > > POOL-426 still pass. I think the race condition is still present > and > >> > the > >> > > > general problem in POOL-413 remains unresolved. > >> > > > > >> > > > On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz < > [email protected]> > >> > wrote: > >> > > > > >> > > >> The fix for POOL-425 included in the 2.13.0 release introduced a > >> > > >> regression that makes addObject no-op when maxIdle is set to a > >> > negative > >> > > >> value (no limit). The POOL-425 fix also failed to account for a > race > >> > > >> condition reported in POOL-426. > >> > > >> > >> > > >> I have created a PR > https://github.com/apache/commons-pool/pull/452 > >> > that addresses > >> > > >> both issues. To avoid the race condition, I had to add > >> > synchronization to > >> > > >> addObject. I tried several ways to avoid the race by modifying > >> > create (as > >> > > >> suggested by Raju Gupta, the OP for POOL-426) but I could not > find a > >> > way to > >> > > >> do that safely without introducing other issues. I don't see the > >> > added > >> > > >> sync in addObject as critical as this method is not used in hot > code > >> > paths > >> > > >> internally and the lock that it acquires is the same lock that > create > >> > will > >> > > >> subsequently acquire if it proceeds to add an object. > >> > > >> > >> > > >> The regression could be addressed in a simpler way by just > fixing the > >> > > >> error in the code (failure to check for negative maxIdle). If > there > >> > are > >> > > >> any doubts about the PR above, I am happy to make that simple > >> > change. In > >> > > >> any case, we should patch this quickly as it will likely break > some > >> > apps > >> > > >> that use addObject with maxIdle unilimited. > >> > > >> > >> > > >> Thanks, all, and sorry for my mistake in the POOL-425 fix. > >> > > >> > >> > > >> Phil > >> > > >> > >> > > > > >> > > >> > - > >> > To unsubscribe, e-mail: [email protected] > >> > For additional commands, e-mail: [email protected] > >> > > >> > > > - > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
Re: Regression in Pool 2.13.0
Hi Phil and all: If I apply https://github.com/apache/commons-pool/pull/453.diff locally, running the test from Eclipse fails, and a Maven (default goal) build fails on random repetition items, for example: https://gist.github.com/garydgregory/fdfbb61360813326998c0e253b10ed52 I assume that we want to proceed with an RC to address the regression, and we are pushing this out to future work. Gary On Sun, Dec 28, 2025 at 7:37 AM Gary Gregory wrote: > > > > > On Sat, Dec 27, 2025, 18:09 Phil Steitz wrote: >> >> I updated the changelog and I think we are ready for a patch release. > > > OK sounds good. I'll get to that today. > > Gary > >> >> Phil >> >> On Sat, Dec 27, 2025 at 12:24 PM Gary Gregory >> wrote: >> >> > That all sounds good. I can create a release candidate anytime if you want. >> > >> > Gary >> > >> > On Sat, Dec 27, 2025 at 1:43 PM Phil Steitz wrote: >> > > >> > > Given that the regression reported in POOL-427 is significant, I think we >> > > should move quickly to validate the fix for the regression (or revert >> > back >> > > to the previous version of the method) and create a patch release as soon >> > > as possible. The investigations around POOL-413 are great and should >> > > continue in parallel. It would be great if we could discuss ideas for >> > how >> > > to address the core issue there here instead of spread across PRs. >> > > >> > > Phil >> > > >> > > On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz >> > wrote: >> > > >> > > > I just reverted the added sync in PR #452, which violates the "no >> > factory >> > > > methods while holding locks" invariant. Strangely, the added tests for >> > > > POOL-426 still pass. I think the race condition is still present and >> > the >> > > > general problem in POOL-413 remains unresolved. >> > > > >> > > > On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz >> > wrote: >> > > > >> > > >> The fix for POOL-425 included in the 2.13.0 release introduced a >> > > >> regression that makes addObject no-op when maxIdle is set to a >> > negative >> > > >> value (no limit). The POOL-425 fix also failed to account for a race >> > > >> condition reported in POOL-426. >> > > >> >> > > >> I have created a PR https://github.com/apache/commons-pool/pull/452 >> > that addresses >> > > >> both issues. To avoid the race condition, I had to add >> > synchronization to >> > > >> addObject. I tried several ways to avoid the race by modifying >> > create (as >> > > >> suggested by Raju Gupta, the OP for POOL-426) but I could not find a >> > way to >> > > >> do that safely without introducing other issues. I don't see the >> > added >> > > >> sync in addObject as critical as this method is not used in hot code >> > paths >> > > >> internally and the lock that it acquires is the same lock that create >> > will >> > > >> subsequently acquire if it proceeds to add an object. >> > > >> >> > > >> The regression could be addressed in a simpler way by just fixing the >> > > >> error in the code (failure to check for negative maxIdle). If there >> > are >> > > >> any doubts about the PR above, I am happy to make that simple >> > change. In >> > > >> any case, we should patch this quickly as it will likely break some >> > apps >> > > >> that use addObject with maxIdle unilimited. >> > > >> >> > > >> Thanks, all, and sorry for my mistake in the POOL-425 fix. >> > > >> >> > > >> Phil >> > > >> >> > > > >> > >> > - >> > To unsubscribe, e-mail: [email protected] >> > For additional commands, e-mail: [email protected] >> > >> > - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: Regression in Pool 2.13.0
On Sat, Dec 27, 2025, 18:09 Phil Steitz wrote: > I updated the changelog and I think we are ready for a patch release. > OK sounds good. I'll get to that today. Gary > Phil > > On Sat, Dec 27, 2025 at 12:24 PM Gary Gregory > wrote: > > > That all sounds good. I can create a release candidate anytime if you > want. > > > > Gary > > > > On Sat, Dec 27, 2025 at 1:43 PM Phil Steitz > wrote: > > > > > > Given that the regression reported in POOL-427 is significant, I think > we > > > should move quickly to validate the fix for the regression (or revert > > back > > > to the previous version of the method) and create a patch release as > soon > > > as possible. The investigations around POOL-413 are great and should > > > continue in parallel. It would be great if we could discuss ideas for > > how > > > to address the core issue there here instead of spread across PRs. > > > > > > Phil > > > > > > On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz > > wrote: > > > > > > > I just reverted the added sync in PR #452, which violates the "no > > factory > > > > methods while holding locks" invariant. Strangely, the added tests > for > > > > POOL-426 still pass. I think the race condition is still present and > > the > > > > general problem in POOL-413 remains unresolved. > > > > > > > > On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz > > wrote: > > > > > > > >> The fix for POOL-425 included in the 2.13.0 release introduced a > > > >> regression that makes addObject no-op when maxIdle is set to a > > negative > > > >> value (no limit). The POOL-425 fix also failed to account for a > race > > > >> condition reported in POOL-426. > > > >> > > > >> I have created a PR https://github.com/apache/commons-pool/pull/452 > > that addresses > > > >> both issues. To avoid the race condition, I had to add > > synchronization to > > > >> addObject. I tried several ways to avoid the race by modifying > > create (as > > > >> suggested by Raju Gupta, the OP for POOL-426) but I could not find a > > way to > > > >> do that safely without introducing other issues. I don't see the > > added > > > >> sync in addObject as critical as this method is not used in hot code > > paths > > > >> internally and the lock that it acquires is the same lock that > create > > will > > > >> subsequently acquire if it proceeds to add an object. > > > >> > > > >> The regression could be addressed in a simpler way by just fixing > the > > > >> error in the code (failure to check for negative maxIdle). If > there > > are > > > >> any doubts about the PR above, I am happy to make that simple > > change. In > > > >> any case, we should patch this quickly as it will likely break some > > apps > > > >> that use addObject with maxIdle unilimited. > > > >> > > > >> Thanks, all, and sorry for my mistake in the POOL-425 fix. > > > >> > > > >> Phil > > > >> > > > > > > > > - > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > >
Re: Regression in Pool 2.13.0
I updated the changelog and I think we are ready for a patch release. Phil On Sat, Dec 27, 2025 at 12:24 PM Gary Gregory wrote: > That all sounds good. I can create a release candidate anytime if you want. > > Gary > > On Sat, Dec 27, 2025 at 1:43 PM Phil Steitz wrote: > > > > Given that the regression reported in POOL-427 is significant, I think we > > should move quickly to validate the fix for the regression (or revert > back > > to the previous version of the method) and create a patch release as soon > > as possible. The investigations around POOL-413 are great and should > > continue in parallel. It would be great if we could discuss ideas for > how > > to address the core issue there here instead of spread across PRs. > > > > Phil > > > > On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz > wrote: > > > > > I just reverted the added sync in PR #452, which violates the "no > factory > > > methods while holding locks" invariant. Strangely, the added tests for > > > POOL-426 still pass. I think the race condition is still present and > the > > > general problem in POOL-413 remains unresolved. > > > > > > On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz > wrote: > > > > > >> The fix for POOL-425 included in the 2.13.0 release introduced a > > >> regression that makes addObject no-op when maxIdle is set to a > negative > > >> value (no limit). The POOL-425 fix also failed to account for a race > > >> condition reported in POOL-426. > > >> > > >> I have created a PR https://github.com/apache/commons-pool/pull/452 > that addresses > > >> both issues. To avoid the race condition, I had to add > synchronization to > > >> addObject. I tried several ways to avoid the race by modifying > create (as > > >> suggested by Raju Gupta, the OP for POOL-426) but I could not find a > way to > > >> do that safely without introducing other issues. I don't see the > added > > >> sync in addObject as critical as this method is not used in hot code > paths > > >> internally and the lock that it acquires is the same lock that create > will > > >> subsequently acquire if it proceeds to add an object. > > >> > > >> The regression could be addressed in a simpler way by just fixing the > > >> error in the code (failure to check for negative maxIdle). If there > are > > >> any doubts about the PR above, I am happy to make that simple > change. In > > >> any case, we should patch this quickly as it will likely break some > apps > > >> that use addObject with maxIdle unilimited. > > >> > > >> Thanks, all, and sorry for my mistake in the POOL-425 fix. > > >> > > >> Phil > > >> > > > > > - > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
Re: Regression in Pool 2.13.0
That all sounds good. I can create a release candidate anytime if you want. Gary On Sat, Dec 27, 2025 at 1:43 PM Phil Steitz wrote: > > Given that the regression reported in POOL-427 is significant, I think we > should move quickly to validate the fix for the regression (or revert back > to the previous version of the method) and create a patch release as soon > as possible. The investigations around POOL-413 are great and should > continue in parallel. It would be great if we could discuss ideas for how > to address the core issue there here instead of spread across PRs. > > Phil > > On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz wrote: > > > I just reverted the added sync in PR #452, which violates the "no factory > > methods while holding locks" invariant. Strangely, the added tests for > > POOL-426 still pass. I think the race condition is still present and the > > general problem in POOL-413 remains unresolved. > > > > On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz wrote: > > > >> The fix for POOL-425 included in the 2.13.0 release introduced a > >> regression that makes addObject no-op when maxIdle is set to a negative > >> value (no limit). The POOL-425 fix also failed to account for a race > >> condition reported in POOL-426. > >> > >> I have created a PR https://github.com/apache/commons-pool/pull/452 that > >> addresses > >> both issues. To avoid the race condition, I had to add synchronization to > >> addObject. I tried several ways to avoid the race by modifying create (as > >> suggested by Raju Gupta, the OP for POOL-426) but I could not find a way to > >> do that safely without introducing other issues. I don't see the added > >> sync in addObject as critical as this method is not used in hot code paths > >> internally and the lock that it acquires is the same lock that create will > >> subsequently acquire if it proceeds to add an object. > >> > >> The regression could be addressed in a simpler way by just fixing the > >> error in the code (failure to check for negative maxIdle). If there are > >> any doubts about the PR above, I am happy to make that simple change. In > >> any case, we should patch this quickly as it will likely break some apps > >> that use addObject with maxIdle unilimited. > >> > >> Thanks, all, and sorry for my mistake in the POOL-425 fix. > >> > >> Phil > >> > > - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: Regression in Pool 2.13.0
Given that the regression reported in POOL-427 is significant, I think we should move quickly to validate the fix for the regression (or revert back to the previous version of the method) and create a patch release as soon as possible. The investigations around POOL-413 are great and should continue in parallel. It would be great if we could discuss ideas for how to address the core issue there here instead of spread across PRs. Phil On Sat, Dec 27, 2025 at 11:10 AM Phil Steitz wrote: > I just reverted the added sync in PR #452, which violates the "no factory > methods while holding locks" invariant. Strangely, the added tests for > POOL-426 still pass. I think the race condition is still present and the > general problem in POOL-413 remains unresolved. > > On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz wrote: > >> The fix for POOL-425 included in the 2.13.0 release introduced a >> regression that makes addObject no-op when maxIdle is set to a negative >> value (no limit). The POOL-425 fix also failed to account for a race >> condition reported in POOL-426. >> >> I have created a PR https://github.com/apache/commons-pool/pull/452 that >> addresses >> both issues. To avoid the race condition, I had to add synchronization to >> addObject. I tried several ways to avoid the race by modifying create (as >> suggested by Raju Gupta, the OP for POOL-426) but I could not find a way to >> do that safely without introducing other issues. I don't see the added >> sync in addObject as critical as this method is not used in hot code paths >> internally and the lock that it acquires is the same lock that create will >> subsequently acquire if it proceeds to add an object. >> >> The regression could be addressed in a simpler way by just fixing the >> error in the code (failure to check for negative maxIdle). If there are >> any doubts about the PR above, I am happy to make that simple change. In >> any case, we should patch this quickly as it will likely break some apps >> that use addObject with maxIdle unilimited. >> >> Thanks, all, and sorry for my mistake in the POOL-425 fix. >> >> Phil >> >
Re: Regression in Pool 2.13.0
I just reverted the added sync in PR #452, which violates the "no factory methods while holding locks" invariant. Strangely, the added tests for POOL-426 still pass. I think the race condition is still present and the general problem in POOL-413 remains unresolved. On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz wrote: > The fix for POOL-425 included in the 2.13.0 release introduced a > regression that makes addObject no-op when maxIdle is set to a negative > value (no limit). The POOL-425 fix also failed to account for a race > condition reported in POOL-426. > > I have created a PR https://github.com/apache/commons-pool/pull/452 that > addresses > both issues. To avoid the race condition, I had to add synchronization to > addObject. I tried several ways to avoid the race by modifying create (as > suggested by Raju Gupta, the OP for POOL-426) but I could not find a way to > do that safely without introducing other issues. I don't see the added > sync in addObject as critical as this method is not used in hot code paths > internally and the lock that it acquires is the same lock that create will > subsequently acquire if it proceeds to add an object. > > The regression could be addressed in a simpler way by just fixing the > error in the code (failure to check for negative maxIdle). If there are > any doubts about the PR above, I am happy to make that simple change. In > any case, we should patch this quickly as it will likely break some apps > that use addObject with maxIdle unilimited. > > Thanks, all, and sorry for my mistake in the POOL-425 fix. > > Phil >
