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 >> >
