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]
