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 <[email protected]> wrote: > On Mon, Dec 29, 2025 at 8:06 AM Gary Gregory <[email protected]> > wrote: > > > > On Sun, Dec 28, 2025 at 12:44 PM Phil Steitz <[email protected]> > 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: <true> but was: <false> > 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 <[email protected]> > 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 <[email protected] > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Dec 27, 2025, 18:09 Phil Steitz <[email protected]> > 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 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] > >
